-
Notifications
You must be signed in to change notification settings - Fork 50
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
Build updates #148
Build updates #148
Conversation
Update checkout to avoid deprecation warnings and skip rebuilding just for documentation updates
Remove CentOS7 stability flag to check if it's just that OS failing actions/checkout@v4
Hello @hdholm. Thanks for the PR. From my perspective, we could keep CentOS7 until it is EOL, and them remove it from the matrix. However, I don´t understand what you mean with this: |
What I was trying to convey was that while CentOS 7 is still in the matrix in this patch, it doesn't actually build anymore. So it kind of seems pointless to leave it there. But the only way to make it build is to not do anything - and everything will fail when Node 16 goes away - or split the builds into CentOS 7 with v3 and everything else with v4. But that is only a stopgap measure because it seems like v3 won't be around through CentOS 7 EOL. i.e., the something that will have to change is v4 getting added to all the other builds either with this patch or another. It just wasn't clear to me that leaving CentOS 7 in had any real benefit to anyone given the choices. I hope that's more clear. It feels like I'm explaining my thinking badly here. |
Hello @hdholm. I understand your point now, thanks for clarifying. IMHO, I would directly remove the CentOS7 compile/test option in this PR. Once we merge it, if someone requires to include CentOS7 at CI level, we can discuss what to do. |
Remove CentOS7 since it's nearing EOL and fails to build using actions/checkout@v4 and v3 will soon not be an option.
I removed CentOS 7 entirely given it's pending EOL and inability to support the Node 20 checkout changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM
This does two straightforward things. One, stop doing mass rebuilds when only the documentation has changed, and two update to actions/checkout@v4 in reaction to https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ which will obsolete actions/checkout@v3. Unfortunately, the update to checkout (and thus node 20) seems to require libraries not available in CentOS 7. I therefore marked it as unstable in the config to keep its failure from stopping all other builds. But I'm really not certain what is the best way to proceed. It seems like checkout v3 will go end-of-life and not be usable "spring 2024" from the blog entry above. CentOS 7 is scheduled to go end-of-life at the end of June. So it seems like something will have to change before CentOS 7 is EOL to keep all the other builds working. The solution here, is probably not ideal, but the choices seem to be remove CentOS 7 altogether or duplicate a bunch of build script to use checkout v3 for CentOS 7. But that would only save it for a few months, still not quite to the end of June. So marking it unstable is a half-measure to raise the issue until there's a better decision.