-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add 24.04 images, override -latest with -24.04 #5951
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
base: master
Are you sure you want to change the base?
Conversation
unfortunately the repo with the images hasn't updated their -latest tag in the last year, so we explicitly set it here.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (11.76%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #5951 +/- ##
==========================================
+ Coverage 74.65% 74.74% +0.09%
==========================================
Files 73 73
Lines 11139 11211 +72
==========================================
+ Hits 8316 8380 +64
- Misses 2186 2197 +11
+ Partials 637 634 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ChristopherHX
left a comment
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.
Look (almost) good to me, see comments
This is not going to update the actrc files of any existing user, asked when empty then written once
FYI, if you see no merge in near future
- I am unable to merge anything right now without owner override of cplee
- Maintainer merge is broken, due to mergify breaking changes that I tried to fix in three weeks ago #5944, still unable to merge dependency updates that had automerge permission
|
Don't want this go out of scope, but maybe there should be a warning in case someone has their |
ChristopherHX
left a comment
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.
Looks good to me
|
Wow you were faster than me. I added the warning that @Andrej730 asked for, but I am not sure it's where you'd like it to be. |
|
The warning is wrong, as it will always trigger when using latest image |
I think you were right that there was an issue, but it was a different issue. The issue this PR solves is that catthehacker/ubuntu seems more or less unmaintained and However, so far the warning only came for one of the image variants. I added checks for all of the default images. I also got rid of the HasPrefix - I thought this is needed because the string might contain a newline that would mess up a simple equality check, but now I see we are stripping arg of whitespace after taking it from the scanner. |
Well we could fix this, we both have write access to the mentioned repository, BTW I am a bit sceptical about everything you changed after my approval. |
|
Happy to revert all of the warning stuff, just wanted to make Andrej happy. I suppose the cleaner fix would be to update the container tag, but I can't do that in a PR. |
|
The warning is not critical or anything, just wanted to avoid people stumbling upon the same issue and creating issues on why |
|
|
|
bumped full-latest as well in the same repo |
|
This can be closed? |
|
Should be updated to not alert on latest tag and preserve them
only this should be reverted |
Unfortunately the repo with the images hasn't updated their -latest tag in the last year, so we explicitly set it here.
Closes #2329