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: update pre-commit image to current ubuntu latest #152

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

ZLLentz
Copy link
Member

@ZLLentz ZLLentz commented Nov 6, 2024

The ubuntu-20.04 image runs python 3.8 which is too old for some of the twincat pre-commit builds.

Rather than keep using the old image, I opted to update to the 22.04 image that is currently tagged as "ubuntu-latest".
I could have also chosen 24.04 but since it wasn't tagged as latest I decided to only go up to 22.04.

List of builds:
https://github.com/actions/runner-images/tree/main

I then tried to run pre-commit on CI on a repo that was previously failing.
Failing build: https://github.com/pcdshub/lcls-twincat-math/actions/runs/11582459856/job/32245532359
My passing build: https://github.com/ZLLentz/lcls-twincat-math/actions/runs/11710367753/job/32616497674

I did not try this again on the python repos but I expect this to be transparent.
I am not currently interested in updating all the builds because that has some (low) risk of creating havoc and I'm not ready for that today.

Ref https://jira.slac.stanford.edu/browse/ECS-6684

@ZLLentz ZLLentz requested a review from tangkong November 6, 2024 19:22
Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

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

This LGTM, though I'm a bit confused how we only ran into this now, after being off 3.8 for years and the offending check being around for 5ish months. I guess we don't run that much twincat CI?

@tangkong
Copy link
Contributor

tangkong commented Nov 6, 2024

Once this is merged I can re-run a python pre-commit test to check. (I conveniently left an atef PR open)

@ZLLentz
Copy link
Member Author

ZLLentz commented Nov 6, 2024

This happened now because Roald added new checks in pcdshub/pre-commit-hooks#29 and we didn't take advantage of them until Nick tried them out in pcdshub/lcls-twincat-math#1.

So, this is currently the only repo of ours using this check, hence only learning about it now.

Any other consumers using this check would have had no issues if they were already using ubuntu-latest.

@ZLLentz ZLLentz merged commit 7702e25 into pcdshub:master Nov 6, 2024
1 check passed
@ZLLentz ZLLentz deleted the fix_pre_commit_tcmath branch November 6, 2024 19:31
@tangkong
Copy link
Contributor

tangkong commented Nov 6, 2024

Ah of course, the pre-commit runs using the repo's config. makes total sense

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