Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

use generateName instead of just name #76

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

sparkoo
Copy link
Member

@sparkoo sparkoo commented Aug 9, 2019

Signed-off-by: Michal Vala mvala@redhat.com

What does this PR do?

updates devfiles to use generateName instead of just name.
Targeted to v7.1.0

What issues does this PR fix or reference?

eclipse-che/che#13794
merge after this PR eclipse-che/che#14157 !!!

Signed-off-by: Michal Vala <mvala@redhat.com>
@ibuziuk ibuziuk changed the title use generateName instead of just name [DON'T MERGE 7.1.0 TARGET] use generateName instead of just name Aug 9, 2019
Copy link
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

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

Why are you including the dash symbol (-) in the generateName?
In the issue comment you say that generateName will be used to generate a name in the form [generateName]-YYYYY. Hence if generateName: angular then the resulting name would be angular-YYYYY. No need to add a dash no?

@sparkoo
Copy link
Member Author

sparkoo commented Aug 12, 2019

@l0rd yes. Current implementation in eclipse-che/che#14157 is without programmed separator and that's how Kubernetes do it. We can do it with separator and I think it would be cleaner. What do you think @sleshchenko @skabashnyuk @benoitf ?

@skabashnyuk
Copy link
Contributor

I would stay to k8s api as close as possible. For example. https://kubernetes.io/docs/concepts/workloads/controllers/replicaset/ . That means if a user wants "-" he has to explicitly add it.

@sleshchenko
Copy link
Member

@sparkoo I'm OK with the current implementation where a user should include separator in generateName if he needs it (K8s API style).

@benoitf
Copy link
Contributor

benoitf commented Aug 12, 2019

I prefer k8s implementation (but only update doc saying that it's [generateName]YYYYY and not [generateName]-YYYYY)

BTW we could still use in examples generateName without a leading dash as well

as a user I mostly don't care to have for my workspace named che1234 or che-1234or che_1234`

but also in k8s examples most of generateName have leading -

@sparkoo sparkoo changed the title [DON'T MERGE 7.1.0 TARGET] use generateName instead of just name use generateName instead of just name Aug 15, 2019
@sleshchenko sleshchenko merged commit 92019b6 into eclipse-che:master Aug 22, 2019
@sparkoo sparkoo deleted the generateName branch August 22, 2019 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants