Skip to content

feat: support application names more than 63 chars long#4051

Closed
YGwen wants to merge 6 commits intoargoproj:masterfrom
YGwen:master
Closed

feat: support application names more than 63 chars long#4051
YGwen wants to merge 6 commits intoargoproj:masterfrom
YGwen:master

Conversation

@YGwen
Copy link

@YGwen YGwen commented Aug 5, 2020

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Optional. My organization is added to USERS.md.
  • I've signed the CLA and my build is green (troubleshooting builds).

@CLAassistant
Copy link

CLAassistant commented Aug 5, 2020

CLA assistant check
All committers have signed the CLA.

@YGwen YGwen changed the title enhancement: Support Application names more than 63 chars long enhancement: support application names more than 63 chars long Aug 5, 2020
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2020

Codecov Report

Merging #4051 into master will decrease coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4051      +/-   ##
==========================================
- Coverage   42.42%   42.33%   -0.10%     
==========================================
  Files         123      123              
  Lines       18229    18232       +3     
==========================================
- Hits         7734     7718      -16     
- Misses       9493     9516      +23     
+ Partials     1002      998       -4     
Impacted Files Coverage Δ
util/settings/settings.go 42.56% <100.00%> (+0.26%) ⬆️
controller/appcontroller.go 48.61% <0.00%> (-2.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb7fa39...592f603. Read the comment docs.

@YGwen YGwen changed the title enhancement: support application names more than 63 chars long feat: support application names more than 63 chars long Aug 5, 2020
@jessesuen
Copy link
Member

I don't think you'll be able to hash the app name since you lose information about the app name which is required for other purposes. I think the approach that you will need to take, is that we allow the app name to be annotated on the resource instead of labeled.

@drewboswell
Copy link
Contributor

My understanding was that the label was only used for synchronizing purposes though. Have you found places that use the label and expect a match between it and the app name?

@drewboswell
Copy link
Contributor

@jessesuen both @YGwen and I see no reason why this fix would not work. Can you show anything to the contrary?

This seems like a quick-win intermediate solution for longer names.

@sbose78
Copy link
Contributor

sbose78 commented Apr 1, 2021

This could either be closed or revived to use the new approach?

@vsfedorenko
Copy link

vsfedorenko commented Jun 30, 2021

I suggest to use <first-48-symbols>-<16 symbols hash> or <first-58-symbols>-<6 chars hash> because otherwise it becomes harder to know what app is behind the name if it consists only of hash.

@alexmt
Copy link
Collaborator

alexmt commented Oct 4, 2021

Superseded by #7251

@alexmt alexmt closed this Oct 4, 2021
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.

8 participants