-
Notifications
You must be signed in to change notification settings - Fork 0
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
tidy up the template questions #10
Conversation
701eaaa
to
05bc9a3
Compare
What is this? I think this is better. The only thing I would question is that it is halfway between being generic and being DLS-specific. I don't think this would be used by an external site as is, so we could either make it more convenient to use at DLS (more strict defaults instead of saying "at DLS ..."), or make it completely generic and then probably write some internal documentation to know what to use at DLS. |
copier.yml
Outdated
@@ -147,11 +136,11 @@ services_repo: | |||
|
|||
services_release: | |||
type: str | |||
help: The initial release of the services repository to track e.g. "2024.1.1" | |||
help: The initial release of the services repository to track e.g. "2024.12.1" or "main" |
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.
Im second guessing the use of "initial". Should updates to the release not be done by a copier update as best practice?
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 dont think so - this is just what you will start with. I think that is what Initial means ?
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.
Yes that is currently what is implied - but the question is should it? Should we not encourage updating the release rather through a copier update?
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 feel that dirties the idea of what the template is for. But that's because I've not considered using this way before. But I think I prefer the idea that ec updates what we track, the template updates what the generic parts of the repo look like.
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.
Thats fine. Its not used in more places so copier probably wont break. I imagine ec only updating on a service level though so its two different places
I think this is a valid point and I considered switching to totally DLS specific for this for the moment. We could always make a new more generic version if anyone ever ends up adopting. Making it DLS specific removes around half of the questions But I have tried to make epics-containers site agnostic - I think it is fair to have 'at DLS we would use this' because it is a useful example to DLS people and outside users. So I vote for sticking with what this PR has:
@GDYendell @marcelldls let me know if you feel differently. |
IT is the Graylog URL used when you type |
I agree that the PR does this. Its a decent middle ground. Changes LGTM |
Thanks @marcelldls. Merging. |
simplify the questions and add more help.
Points to note.
Target namespace, Apps namespace and Argo CD project are all the same string at DLS so I condensed down into a single question. If any external users want to set up ArgoCD we can recommend they use the same approach or generalize this template as necessary.
tested against i04 and p47.
Fixes #9
example interaction: