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

Add publishing workflow; update workflow, dependencies, tasks, docs #147

Merged
merged 44 commits into from
Nov 7, 2023

Conversation

danyalaytekin
Copy link
Member

@danyalaytekin danyalaytekin commented Oct 26, 2023

This PR adds a publishing workflow and makes several other updates. It's designed to be the final rehabilitation PR before the versioning PR for [email protected], which will be the final minor version before 5 (which will use pa11y@7).

Changes

  1. New publishing workflow, publish.yml: this is mostly a duplicate of the common implementation described in:
    • Add publishing workflow pa11y-lint-config#6
    • One difference is that the publishing here takes place with Node 12, which is the minimum version pa11y-webservice@4 supports. A higher version would probably be fine but we're still on lockfile v1 here. Thanks for making me think about this @jpw.
  2. tests.yml workflow updates
    • trigger on push to main instead of master
    • test for all pull request targets
    • remove linter optimisation (explanation in pa11y#664)
    • use npm ci instead of npm i
    • move higher the steps which don't require the server, to allow them to fail earlier, should they fail
    • a few of the changes above were smuggled into Upgrade vulnerable dependencies fixing npm audit reports #145 while trying to get it into shape to be merged before this one - for example npm ci can already be seen to be here
  3. Makefiles:
    • some generalised items removed, plus the removal of make ci, which wasn't being used by CI
    • this does affect npm test since lint has moved into npm run lint (these weren't being used by CI either and this all might be updated further for v5)
    • if you think the loss of linting from npm test constitutes a breaking change please let me know and I'll restore it for this minor version. @hollsk @josebolos
  4. Tests:
    • integration tests will now fail when there's no webservice
    • test runs now require tests to exist
  5. README.md: substantially rewritten, including:
    • Removed warning about Node 16, since we've been testing it in CI
    • Bring configuration discussions into one region
    • New support policy
    • Travis badge fixes
  6. Dependency upgrades including pally-lint-config@3, all minor compared to @sudheesh001's work earlier

master has also been renamed to main in settings as part of:

@danyalaytekin danyalaytekin changed the title Add publishing workflow; update testing workflow [WIP] Add publishing workflow; bring repo up-to-date for final v4 release Nov 7, 2023
@danyalaytekin danyalaytekin changed the title [WIP] Add publishing workflow; bring repo up-to-date for final v4 release Add publishing workflow; update workflow, dependencies, tasks, docs Nov 7, 2023
@danyalaytekin danyalaytekin marked this pull request as ready for review November 7, 2023 16:39
Copy link
Member

@hollsk hollsk left a comment

Choose a reason for hiding this comment

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

Chunky! But all looks very sensible to me.

R.e. whether moving linting from npm test to npm run lint would be a breaking change, I don't think so. Even if it was a big change (like removing it entirely or whatever), we don't get enough contributions to webservice for anyone to actually notice it, IMO. I'll defer to the two Js if they have a different opinion, though.

@danyalaytekin
Copy link
Member Author

Thanks @hollsk. I do think the npm signature change here is a development-related change, since these tasks themselves depend on dev dependencies, although, if I was to find myself face to face with Saint Semver, come to fault my ruining the dreams of people running npm test in prod to obtain the side-effect of an ESLint run, that would be frightening, I guess. I still think it's ok and I will merge with your approval here but if the Js are indignant we can revisit this before 4.2 releases.

@danyalaytekin danyalaytekin merged commit 59bf0f9 into main Nov 7, 2023
3 checks passed
@danyalaytekin danyalaytekin deleted the publishing-workflow branch November 7, 2023 23:32
@danyalaytekin danyalaytekin added this to the 4.2 milestone Nov 7, 2023
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.

2 participants