Skip to content
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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update README.md #173

wants to merge 5 commits into from

Conversation

ptmkenny
Copy link

The correct npm package for the HTML reporter is pa11y-ci-html-reporter, not pa11y-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.

@@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

@hollsk
Copy link
Member

hollsk commented Feb 22, 2022

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?

@joeyciechanowicz
Copy link
Member

These are some lovely rewrites @ptmkenny .

wary of linking to 3rd party packages in our documentation

@hollsk totally agree. Though if the docs make it clear that this is not maintained by pa11y?

"Example of adding GitLabs npm module as a reporter:"

The big takeaway is definitely that the reporters are broken though 😱

@aarongoldenthal
Copy link
Contributor

@hollsk @joeyciechanowicz I'll throw in my agreement for ensuring responsibilities are understood. There have been examples where issues were opened here for pa11y-ci-reporter-html (e.g. #169). I do think there could be value in maybe linking to related projects, with an explanation of responsibility or lack thereof (e.g. I also publish pa11y-ci-reporter-cli-summary, which started as a test project for the reporter interface, and there are likely other related tools). I'm not sure if the right place is here, the pa11y.org website, or somewhere else (an Awesome Pa11y page?).

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 pa11y-reporter-html in the example here may add to the confusion even once working (maybe update to link to the pa11y HTML reporter).

@revolunet
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants