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

justfile: Truncate tag names, everywhere necessary #66

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Nov 9, 2024

  • Revert "justfile: Truncate container tag string if too long"
  • justfile: Truncate container tag string if too long (2nd attempt)

@qmonnet qmonnet requested a review from daniel-noland November 9, 2024 01:51
@qmonnet
Copy link
Member Author

qmonnet commented Nov 9, 2024

Not good yet

@qmonnet qmonnet marked this pull request as draft November 9, 2024 02:08
@qmonnet qmonnet force-pushed the pr/qmonnet/cant-believe-i-am-using-such-a-long-long-branch-name-but-gotta-try-to-overflow-container-tag-name-limit-yes-we-do branch 3 times, most recently from e37c2b9 to 54311ab Compare November 9, 2024 03:03
Copy link
Collaborator

@daniel-noland daniel-noland left a comment

Choose a reason for hiding this comment

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

Maybe use truncate_str as the function name?

justfile Outdated Show resolved Hide resolved
@qmonnet qmonnet force-pushed the pr/qmonnet/cant-believe-i-am-using-such-a-long-long-branch-name-but-gotta-try-to-overflow-container-tag-name-limit-yes-we-do branch from 54311ab to 1246934 Compare November 9, 2024 03:13
@qmonnet qmonnet requested a review from daniel-noland November 9, 2024 03:14
@qmonnet qmonnet marked this pull request as ready for review November 9, 2024 03:14
@qmonnet
Copy link
Member Author

qmonnet commented Nov 9, 2024

All tests passed this time! 🎉

Copy link
Collaborator

@daniel-noland daniel-noland left a comment

Choose a reason for hiding this comment

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

Yup. 💦 Spritzes holy water on commits 💦

This reverts commit 0874727.

Turns out this commit missed a lot of occurrences where tag can be too
long, let's find a more generic truncate function to solve the issue.

Signed-off-by: Quentin Monnet <[email protected]>
Containers' tags can't be longer than 128 characters. Sometimes CI
generates tags that are longer than this (thanks Dependabot for the
extra-long branch names).

Make sure we don't break docker builds by having bash truncate the
string if necessary - everywhere we need it.

Fixes: b88ec42 ("Build system")
Signed-off-by: Quentin Monnet <[email protected]>
@daniel-noland daniel-noland force-pushed the pr/qmonnet/cant-believe-i-am-using-such-a-long-long-branch-name-but-gotta-try-to-overflow-container-tag-name-limit-yes-we-do branch from 1246934 to 728171e Compare November 9, 2024 03:29
@qmonnet
Copy link
Member Author

qmonnet commented Nov 9, 2024

Side note, your preference is to always rebase on the current main? I find that the automatic rebase usually does it well if there's no conflict, and it avoids going again through the CI

@daniel-noland
Copy link
Collaborator

Side note, your preference is to always rebase on the current main? I find that the automatic rebase usually does it well if there's no conflict, and it avoids going again through the CI

As long as we have

  1. all commits tested and passing CI (or at least building)
  2. linear history

then I'm good.

I love bisection methods and non-linear history and commits which don't build ruin it 🤷‍♀️

Really, we should open source this stuff and then we can just use the merge queue method

@qmonnet
Copy link
Member Author

qmonnet commented Nov 9, 2024

all commits tested and passing CI

We don't currently build each commit independently, do we?

@daniel-noland
Copy link
Collaborator

all commits tested and passing CI

We don't currently build each commit independently, do we?

Not yet no. It is something I would like to get in place tho.

I'm less stressed about it very early on in the project like this because it is unlikely we will care about bisection before we implement any real features.

@daniel-noland daniel-noland merged commit 782d9b3 into main Nov 9, 2024
10 checks passed
@daniel-noland daniel-noland deleted the pr/qmonnet/cant-believe-i-am-using-such-a-long-long-branch-name-but-gotta-try-to-overflow-container-tag-name-limit-yes-we-do branch November 9, 2024 03:42
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