Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

update all dependencies to @latest, drop node@8 CI testing, add node@16 #585

Merged
merged 7 commits into from
Nov 6, 2021

Conversation

broofa
Copy link
Member

@broofa broofa commented Oct 23, 2021

Got paranoid after seeing this news and decided to update all our dependencies to the latest version of things.

npm run build and npm test appear to work.

Edit: npm run lint broke. Fixing this involved switching to @eslint/babel-parser, and then eslint --fix and prettier -w to patch formatting differences.

... and apparently jest@latest doesn't work with node@8. So I've updated our CI platforms to drop node 8 and add node 16.

Note: I don't know why Github is saying it needs the "ci (8.x) Expected" check here, since I removed node 8 from the node-versions in the ci.yml workflow. 😕

@broofa broofa marked this pull request as draft October 23, 2021 13:37
@broofa broofa changed the title update all dependencies to @latest update all dependencies to @latest, drop node@8 CI testing, add node@16 Oct 23, 2021
@broofa broofa requested a review from ctavan October 23, 2021 15:45
@broofa broofa marked this pull request as ready for review October 23, 2021 15:45
@ctavan
Copy link
Member

ctavan commented Oct 23, 2021

@broofa you need to adjust the branch settings on GitHub for this repo since I marked all checks as mandatory.

I was tempted to update jest a couple of times in the past but then realized it would technically force us to drop node 8 support.

I do think we should finally drop node 8 support, but again, technically it will require us to do a major version bump…

@broofa
Copy link
Member Author

broofa commented Oct 29, 2021

Okay, removed node 8 from checks. Also renamed master to main.

@broofa broofa mentioned this pull request Nov 4, 2021
@broofa
Copy link
Member Author

broofa commented Nov 5, 2021

@ctavan ping

Copy link
Member

@ctavan ctavan left a comment

Choose a reason for hiding this comment

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

I'm generally fine with this change, but please make sure that when you squash and merge, the resulting commit message follows https://www.conventionalcommits.org/en/v1.0.0/ to produce a breaking change entry in the changelog (e.g. like daf72b8).

I also noticed that the README still needs an update in the summary where it still mentions Node 8.

@broofa broofa merged commit 343e031 into main Nov 6, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants