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.

Sign up for GitHub

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

Jump to bottom

ci: bump actions/checkout version and fix gcc 5 testing #3268

Merged
merged 2 commits into from
Nov 27, 2024
Merged

ci: bump actions/checkout version and fix gcc 5 testing #3268

merged 2 commits into from
Nov 27, 2024

Conversation

dkm
Copy link
Member

@dkm dkm commented Nov 26, 2024 •
edited
Loading

v3 looks obsolete, bump it to the current v4.

Use a GCC 5.4.0 built from Compiler Explorer and execute everything on
ubuntu 22.04.
Use the caching framework provided by github to avoid downloading every
time from Compiler Explorer's servers.

@dkm dkm requested a review from P-E-P November 26, 2024 10:34
@dkm dkm self-assigned this Nov 26, 2024
@dkm dkm force-pushed the topic/bump_checkout branch 12 times, most recently from 850404d to ef2419a Compare November 27, 2024 10:45
@dkm dkm changed the title ci: bump actions/checkout version ci: bump actions/checkout version and fix gcc 5 testing Nov 27, 2024
@dkm
Copy link
Member Author

dkm commented Nov 27, 2024

@P-E-P and @CohenArthur this should be in a functional state now. As mentioned earlier, I think the workflows could benefit from a cleanup/refactoring.

@dkm dkm force-pushed the topic/bump_checkout branch from ef2419a to c655bfb Compare November 27, 2024 10:49
@dkm
ci: bump actions/checkout from v3 to v4
865d5af 
ChangeLog:

	* .github/workflows/Remark.yml: Use v4.
	* .github/workflows/bootstrap.yml: Likewise.
	* .github/workflows/ccpp.yml: Likewise.
	* .github/workflows/ccpp32alpine.yml: Likewise.
	* .github/workflows/clang-format.yml: Likewise.
	* .github/workflows/commit-format.yml: Likewise.
	* .github/workflows/docker.yml: Likewise.

Signed-off-by: Marc Poulhiès <[email protected]>
philberty
philberty approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

I love this.

Only question we have is do we want to make this a requirement for merging? Like we had with the gcc 4.8 one i sort of dont think its worth making it a requirement. but 99.99% of the time people will see the CI job fail and just fix it anyway

@dkm
ci: change the gcc-5 testing
a2ebcbc 
Use a GCC 5.4.0 built from Compiler Explorer and execute everything on
ubuntu 22.04.
Use the caching framework provided by github to avoid downloading every
time from Compiler Explorer's servers.

ChangeLog:

	* .github/workflows/ccpp.yml: Adjust to use gcc-5 on ubuntu 22.04.

Signed-off-by: Marc Poulhiès <[email protected]>
@dkm dkm force-pushed the topic/bump_checkout branch from c655bfb to a2ebcbc Compare November 27, 2024 10:57
@dkm
Copy link
Member Author

dkm commented Nov 27, 2024

I love this.

Only question we have is do we want to make this a requirement for merging? Like we had with the gcc 4.8 one i sort of dont think its worth making it a requirement. but 99.99% of the time people will see the CI job fail and just fix it anyway

oh, this, I don't know. I think you're more qualified than me to decide (along with the real devs, doing actual coding)

@P-E-P
Copy link
Member

P-E-P commented Nov 27, 2024

Only question we have is do we want to make this a requirement for merging?

Why wouldn't we make a it a requirement ? It shouldnt be more than a box to check in the settings right ? I feel like I'm missing something here.

@dkm
Copy link
Member Author

dkm commented Nov 27, 2024

I love this.

Only question we have is do we want to make this a requirement for merging? Like we had with the gcc 4.8 one i sort of dont think its worth making it a requirement. but 99.99% of the time people will see the CI job fail and just fix it anyway

hmmm, it's currently not required... So, we're ok leaving it as-is I guess?
image

@dkm
Copy link
Member Author

dkm commented Nov 27, 2024

Only question we have is do we want to make this a requirement for merging?

Why wouldn't we make a it a requirement ? It shouldnt be more than a box to check in the settings right ? I feel like I'm missing something here.

I think that's a setting only available to the project's admin. Maybe you are? At least @philberty is and can confirm.... But IIUC, there's nothing more to do in my PR regarding the "required" aspect of gcc-5-build.

@philberty philberty added this pull request to the merge queue Nov 27, 2024
CohenArthur
CohenArthur approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

amazing work, thank you @dkm!!! ❤️❤️❤️

Merged via the queue into master with commit 8d40193 Nov 27, 2024
13 checks passed
@dkm dkm deleted the topic/bump_checkout branch November 27, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur approved these changes

@P-E-P P-E-P P-E-P approved these changes

@philberty philberty philberty approved these changes

Assignees

@dkm dkm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dkm @P-E-P @philberty @CohenArthur