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

Fix Newer NodeJS CI #106

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Fix Newer NodeJS CI #106

merged 3 commits into from
Dec 14, 2023

Conversation

confused-Techie
Copy link
Member

This PR aims to resolve the CI issues we are seeing on this repo.

As activity has been spares over here, seems this repo had never yet received the new setuptools and Python 3.12+ actions logic.

So this PR simply takes what's mostly used over at Pulsar, and adds it's logic to the CI here.

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Thank you for this!!!!!

Technically we can delete the "install Python 3.12" if we want, but this pins it so Python 3.13 (several months away) can't surprise us...

I slightly like to not pin stuff when I can get away with it, to make sure our stuff breaks in CI first rather than have end users wonder why it didn't work for them. So we can catch it early in the repo and fix it early in the repo. But, not having your CI break for reasons outside of your code changes is a benefit of pinning, so.

Will accept this PR as-is, or changed to not manually install Python 3.12 (for the non Node 14 runs).

@confused-Techie
Copy link
Member Author

Thanks for the approval, tbh with the amount of issues Python has given us, I'd much rather add Python versions into the matrix if we wanted to run tests on version above what we know for a fact works.

But with everything else already pinned, I'm inclined to add as is, rather than deal with the surprises. But do totally get where you are coming from. But thanks for adding approval as is, which I'll merge, but am totally not against running tests on this Python version and newer (or latest) if we felt it was a good idea. Thanks!

@confused-Techie confused-Techie merged commit 4a8936a into master Dec 14, 2023
11 checks passed
@confused-Techie confused-Techie deleted the fix-ci branch December 14, 2023 09:45
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