-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 branch slug #10698
fix branch slug #10698
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10698 +/- ##
=======================================
Coverage 45.75% 45.76%
=======================================
Files 236 236
Lines 28527 28533 +6
=======================================
+ Hits 13053 13057 +4
- Misses 13669 13670 +1
- Partials 1805 1806 +1 ☔ View full report in Codecov by Sentry. |
// we need to limit the 'branch' to 50 to give room to append/suffix-ing it | ||
// with 13 more characters. Also, there is the need to clean it as recommended | ||
// we need to limit the 'branch' to 63. | ||
// Also, there is the need to clean it as recommended | ||
// here https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names | ||
slug.MaxLength = 50 | ||
slug.MaxLength = 63 | ||
|
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.
I wonder, is the comment just plainly wrong? It talks about suffixing the slug with 13 additional characters. If we increase the max. length to 63 here, it can't be suffixed anymore.
Can you please elaborate on this change?
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.
I checked the code, i don't find the code for generate
. So I think it is ok to change the max length of branch-slug to 63, and it is was mentioned in #10622.
Fixes: argoproj#10622 Signed-off-by: cleverhu <shouping.hu@daocloud.io>
09ae444
to
948cb5a
Compare
Superseded by #10801 |
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: