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

Use node v14 for CI. #307

Merged
merged 1 commit into from
May 1, 2023
Merged

Use node v14 for CI. #307

merged 1 commit into from
May 1, 2023

Conversation

jgerigmeyer
Copy link
Member

Many of the tools used in CI no longer support node v12. Now that node v14 is the oldest maintained version, this runs CI against that version. (Other changes are unrelated, but came along with a npm run build.)

@LeaVerou
Copy link
Member

LeaVerou commented May 1, 2023

Whoa, 12. Even 14 is kinda old, can we update to 18?

Copy link
Member

@LeaVerou LeaVerou left a comment

Choose a reason for hiding this comment

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

To keep things clean, could you remove the CSS files from this PR? We need to git rm them, we shouldn't have build artifacts in the repo…

@jgerigmeyer
Copy link
Member Author

Whoa, 12. Even 14 is kinda old, can we update to 18?

Hm, I'm torn on that. One of the purposes of CI is to ensure that the package as a whole still works for older Node environments, and v14 is still in active maintenance mode. While I assume development will happen in node v18 or even v20, I think it's reasonable to run CI against an older supported version? It's a bit blurry with a package like this which is also intended for browser usage, but I don't see a good reason to skip node v14 unless/until it starts breaking things. 🤷

To keep things clean, could you remove the CSS files from this PR? We need to git rm them, we shouldn't have build artifacts in the repo…

Done! ✅

@jgerigmeyer jgerigmeyer requested a review from LeaVerou May 1, 2023 01:31
@LeaVerou
Copy link
Member

LeaVerou commented May 1, 2023

Whoa, 12. Even 14 is kinda old, can we update to 18?

Hm, I'm torn on that. One of the purposes of CI is to ensure that the package as a whole still works for older Node environments, and v14 is still in active maintenance mode. While I assume development will happen in node v18 or even v20, I think it's reasonable to run CI against an older supported version? It's a bit blurry with a package like this which is also intended for browser usage, but I don't see a good reason to skip node v14 unless/until it starts breaking things. 🤷

To keep things clean, could you remove the CSS files from this PR? We need to git rm them, we shouldn't have build artifacts in the repo…

Done! ✅

I think it's primarily which version we use to build. Our testsuite currently doesn't even run on Node at all, it's entirely client-side. Anyhow, approving this since 14 is better than 12.

@jgerigmeyer jgerigmeyer merged commit a4fd494 into color-js:main May 1, 2023
@jgerigmeyer jgerigmeyer deleted the hotfix-ci branch May 1, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants