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

ci: Remove CircleCI and update Node versions #141

Merged
merged 4 commits into from
Jun 30, 2023
Merged

Conversation

alexrudd2
Copy link
Contributor

@alexrudd2 alexrudd2 commented Jun 28, 2023

Closes #140.

I have forced an update of the transitive dependency nan in package-lock.json, but avoided listing it as a direct dependency in package.json. Along with updating webpack to v5, this would make the CI green (alexrudd2@725f0ed)...

... if #139 is fixed. So I expect this PR to fail for the time being.

@nornagon
Copy link
Owner

While removing CI does technically result in CI not failing, I'd prefer CI to get closer to passing instead 😅

@alexrudd2
Copy link
Contributor Author

While removing CI does technically result in CI not failing, I'd prefer CI to get closer to passing instead 😅

Oh, sorry I was unclear. This removes CircleCI but retains/updates GitHub Actions. I'm assuming there was no need for both.

@nornagon
Copy link
Owner

Ah, right, yes that makes sense. Thanks!

I've regenerated package-lock.json because as a rule I don't merge package-lock.json changes from untrusted folks for security reasons.

@nornagon nornagon merged commit 00cef01 into nornagon:main Jun 30, 2023
0 of 3 checks passed
@alexrudd2 alexrudd2 deleted the ci branch June 30, 2023 22:34
@alexrudd2
Copy link
Contributor Author

I've regenerated package-lock.json because as a rule I don't merge package-lock.json changes from untrusted folks for security reasons.

OK, fine with me. It actually might save a bit of trouble later in with broken dependencies.

Actually, I've now managed to update ALL the dependencies and fix the CI in my fork: https://github.com/alexrudd2/saxi. (It was a pretty painful introduction to TypeScript 😆 ).

How would you prefer I upstream things? (Note I did revert 593c451, as did other people).

@nornagon
Copy link
Owner

I don't want to accept a revert of 593c451 into this repository. I'm happy to review other reasonable changes though!

@alexrudd2
Copy link
Contributor Author

I don't want to accept a revert of 593c451 into this repository. I'm happy to review other reasonable changes though!

OK. I'll gently ask you to reconsider. It does break the CI, apparently breaks some people's real hardware (#131), and at least one other fork has also reverted.

In any case, I'll see what I can do to apply the fixes I've learned using my fork in manageable PRs.

Are you interested in setting up renovatebot?

@nornagon
Copy link
Owner

The CI breaking is not a fundamental problem with WebSerial support; it's certainly fixable. It's far from clear to me that #131 is a problem with WebSerial per se. If it is, I'd rather find and fix the issue than remove WebSerial support.

I have no idea what renovatebot is or why I'd want it, perhaps you could give a little more info?

@alexrudd2
Copy link
Contributor Author

The CI breaking is not a fundamental problem with WebSerial support; it's certainly fixable.

I was able to fix part of it (with a polyfill), but couldn't get it all the way. I'll send a PR with the partial fix.

It's far from clear to me that #131 is a problem with WebSerial per se. If it is, I'd rather find and fix the issue than remove WebSerial support.

I'll test on real hardware soon and let you know if I can reproduce.

I have no idea what renovatebot is or why I'd want it, perhaps you could give a little more info?

Renovatebot is a more powerful Dependabot. Here's an example PR - it groups similar updates together, and gives full changelogs: alexrudd2#42. I also set it up to automatically merge when the CI passes (with the revert, of course). No affiliation, I'm just impressed.

@nornagon
Copy link
Owner

Eh, no thanks, I like that dependabot is built into github and requires very little thought from me. Mend.io seems like something that quite possibly won't exist in 5 years, which is already less than the lifetime of this project :)

@alexrudd2
Copy link
Contributor Author

Heh, good timing on dropping Node 12.x since GitHub Actions is going to drop it on Aug 14.

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.

CI is broken on newer Node versions
2 participants