-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update README.md #173
base: main
Are you sure you want to change the base?
Update README.md #173
Conversation
@@ -76,11 +78,13 @@ You can use the `--config` command line argument to specify a different file, wh | |||
} | |||
``` | |||
|
|||
Pa11y will be run against each of the URLs in the `urls` array and the paths specified as CLI arguments. Paths can be specified as relative, absolute and as [glob](https://github.com/isaacs/node-glob#glob) patterns. | |||
Pa11y will be run against each URL specified in the `urls` array and ech path specified in the command line arguments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pa11y will be run against each URL specified in the `urls` array and ech path specified in the command line arguments. | |
Pa11y will be run against each URL specified in the `urls` array and each path specified in the command line arguments. |
Thanks so much for doing this, @ptmkenny - your rewrites are great! I guess my only concern with changing the name of the reporter in this documentation is that pa11y-ci-html-reporter wasn't written by the Pa11y team, it belongs to a Pa11y user. I'm a bit wary of linking to 3rd party packages in our documentation because it could give people the impression that we have control over and maintain them. It's not that I think @aarongoldenthal has secretly included a bitcoin miner in his package 🙃 more that we want people to have some clarity over whether a package actually originated from the Pa11y team in the first place so they know where to get support. But then if the HTML reporter that we do support doesn't work in Pa11y CI, it's not exactly useful for us to be directing people to install it in the documentation. Any other Pa11y core folks want to chime in? |
These are some lovely rewrites @ptmkenny .
@hollsk totally agree. Though if the docs make it clear that this is not maintained by pa11y?
The big takeaway is definitely that the reporters are broken though 😱 |
@hollsk @joeyciechanowicz I'll throw in my agreement for ensuring responsibilities are understood. There have been examples where issues were opened here for On the issues with pa11y reporters in pa11y-ci I took a look and captured the issue at #174. One other related thoughts on reporters... The published reporter modules are in a little bit of an intermediate place. The repositories here note that they're deprecate since they're integrated with pa11y, but the npm packages have not been officially deprecated, which might make it clearer. And including the |
Using npx, looks like the reporter is simply "html" ➜ npx pa11y https://google.fr --reporter=pa11y-reporter-html > out.html
Reporter "pa11y-reporter-html" could not be found
➜ npx pa11y https://google.fr --reporter=pa11y-ci-reporter-html > out.html
Reporter "pa11y-ci-reporter-html" could not be found
➜ npx pa11y https://google.fr --reporter=html > out.html
OK |
The correct npm package for the HTML reporter is
pa11y-ci-html-reporter
, notpa11y-html-reporter
. I found the documentation on reporters very confusing (see https://stackoverflow.com/questions/71145049/how-do-i-use-the-html-csv-reporters-in-pa11y-with-github-actions), so I rewrote it to be more clear. I also generally edited the document to be easier to understand.