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

bump eslint-plugin-github to v5.0.1 #190

Merged
merged 4 commits into from
Jun 5, 2024
Merged

Conversation

khiga8
Copy link
Contributor

@khiga8 khiga8 commented Jun 3, 2024

Bumps the dependency, eslint-plugin-github, to the latest major version which includes:

@khiga8 khiga8 requested a review from a team as a code owner June 3, 2024 14:43
@khiga8 khiga8 requested a review from joshblack June 3, 2024 14:43
Copy link

changeset-bot bot commented Jun 3, 2024

🦋 Changeset detected

Latest commit: 726aa88

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-primer-react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@joshblack
Copy link
Member

@khiga8 for v5, do we need to update any deps on our end (or would consumers need to make sure eslint deps are updated)?

@khiga8
Copy link
Contributor Author

khiga8 commented Jun 3, 2024

Hey @joshblack! The main notable change is that we dropped support in eslint-plugin-github for node 14 and node 16 in favor of node 18.

I see that ci.yml is already running node 18 so I don't believe anything needs to change.

@khiga8 khiga8 changed the title bump eslint-plugin-github bump eslint-plugin-github to v5.0.0 Jun 3, 2024
@joshblack
Copy link
Member

@khiga8 ah thanks for explaining the node change. In this case, if someone is using eslint-plugin-primer-react and is using Node 14 or 16 then it would no longer work as expected? (Due to usage of eslint-plugin-github and it no longer supporting these versions)

For context, I'm not looking at this from the perspective of someone using the package and if this would be a breaking change for them or not.

@khiga8
Copy link
Contributor Author

khiga8 commented Jun 4, 2024

Hey @joshblack! Thats a great question.. 👀 .

I dropped node 14 in eslint-plugin-github because I was running into issues with running tests specifically in a node 14 test build. I reached out on slack thread, and it was pointed out that we generally only support Node LTS so it seemed reasonable to drop node 14/16. This seems to align with how node 20 will soon be the the default in GitHub Actions

The SyntaxError we saw in node 14 seemed related to the library test setup, and not the actively shipped code. It is possible the plugin will continue to run as expected for consumers in Node 14/Node 16. However, because we formally dropped node 14 and node 16 from the library testing matrix, we won't be able to guarantee that the plugin will work so I decided to cut a major release version just in case!

If we merge this in, it could be a good idea to cut a major release to be safe, but I did notice that the CI test matrix for eslint-plugin-primer-react only runs node 18 anyways while the CI test for primer/react only runs node 20. Does this mean that technically these libraries are already not formally supporting node <18 🤔 ? Curious to hear your thoughts!

@khiga8 khiga8 force-pushed the kh-bump-eslint-plugin-github branch from eaa1d6c to 56cef34 Compare June 4, 2024 22:31
@khiga8 khiga8 changed the title bump eslint-plugin-github to v5.0.0 bump eslint-plugin-github to v5.0.1 Jun 4, 2024
Copy link
Member

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to explain @khiga8! Would you mind including a changeset? Then should be good to go 💯

@khiga8 khiga8 enabled auto-merge (squash) June 5, 2024 20:19
@khiga8 khiga8 merged commit a02a188 into main Jun 5, 2024
7 checks passed
@khiga8 khiga8 deleted the kh-bump-eslint-plugin-github branch June 5, 2024 20:20
@primer-css primer-css mentioned this pull request Jun 5, 2024
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