-
Notifications
You must be signed in to change notification settings - Fork 169
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
Indicate generated yaml by .gitignore-ing it and including a comment indicating it's generated (to reduce maintenance effort for contributors) #299
Comments
Thanks, @edeandrea. Is this an accurate picture, or is there stuff in |
Not quite. The
The The script also then appends the services to the root Does that make sense? I've also thought about building a docker compose extension that could re-use Dekorate to be able to generate the app specific docker compose files like it does for kubernetes, but I never invested in that (no time unfortunately) and wasn't sure if there are others that would benefit from it either. |
Ah, ok. So the yellow line I proposed is there, but going in the opposite direction? We should perhaps do the same thing I suggested elsewhere, and .gitignore the generated content, and have the script that generates it run in the CI. And also put a comment in the file explaining that it's generated. Hopefully between those three, it might stop people (like me :) ) investing in maintaining those files. The ideal would be not even having them in source control, but that would need an extension, as you say.
It does. When I was working through the So I think then there's no more scripting needed, it's just a question of having the generated files be more clearly labelled as generated, so people don't spend time changing them. The comment has to be in the file itself, because we can't make people cross-reference files they might change against readmes to see if the file is listed in the readme as a generated file. Even this would do the trick, I think, but CI + .gitignore would be more complete:
|
... and ideally also a similar comment in the 'donor' yaml files explaining where that content ends up, and the script strips out that comment ... but that's slightly less trivial to implement. |
How would you envision the |
I'm pretty sure the CI doesn't actually need to commit the change to the echo whatever >> interesting.file
rm .gitignore
git add interesting.file # being specific, here
git commit -m "this only changes the interesting file"
git push would work. It leaves the CI with a dirty git workspace, but we don't care, because it's an ephemeral machine. |
Thats definitely an interesting idea. Worth trying out I think. |
It ends up being "one rule for the CI, one rule for the humans," which is slightly confusing, but in combination with a comment on the file is probably clearest overall. |
Although it still wouldn't have kept you from modifying the files in the first place. Just would have made you scratch your head as to why your changes didn't commit :) |
Although maybe the folder showing up as a different color in the IDE may have given you a clue |
Yes, you'd need the comment on the files themselves to alert people to the fact that hand-editing them is (a) ineffectual and (b) a waste of time. :) |
I'll also have to check because we are not manually doing quarkus-super-heroes/.github/workflows/create-deploy-resources.yml Lines 59 to 66 in 1a847cb
|
See also quarkusio/quarkus#34025 for the verbosity of the config. I'll update this WI title to keep the scope just at the handling of the generated files. |
Hey @holly-cummins I was circling back around to this and I have it mostly working in a test repo. The files look ignored in the IDE, then CI regenerates them, deletes the See this commit for an example. Then in my IDE I see the updates and I can pull them in. Yay! BUT...... What I'm seeing after I pull in the updated files is that the IDE doesn't seem to treat them as ignored anymore, even though the Have you come across anything like this before? |
Actually upon further investigation, it doesn't seem that its the IDE that is the problem. If I do a ╰─ git status
On branch main
Your branch is up to date with 'origin/main'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: rest-villains/deploy/docker-compose/java17.yml
no changes added to commit (use "git add" and/or "git commit -a") |
It seems that |
So I'm not sure what we are trying to achieve is something that is technically possible. I can definitely add something like this to the top of all generated files:
There are ways to tell git locally to ignore local changes to tracked files by doing something like
but then any developer would have to do that on their local machine, which I think is a road we do not want to go down. There are actually 2 options that could get around this a bit: https://www.baeldung.com/git-assume-unchanged-skip-worktree But again, each user would have to do it for themselves because it is working at the local staging area. And then if you were to |
Hmm, annoying. I agree that no matter what, we should add the comment to the generated files. I guess the other option is that we don't have the CI commit the generated files. If we think people will need them when deploying locally, we could move the generation process to happen in a build step, rather than the CI. |
I guess I don't understand what you mean here? The files are there so it is stupid simple to In the case of |
I just merged #316 which adds the comment to the top of the generated headers. |
One thing we could do, which would involve the user actually doing something, is that we could create a script which "untracks" those files. We could include instructions for users on how to register it as a pre-commit hook in their local repo. But we couldn't control whether or not they actually did that. |
I think asking users to run a script undoes the aim of the changes, which is to try and ensure users can 'do the right thing' in the project without having to read lots of documentation. So I think we should leave it with just the comment in the files. |
I agree. I tend to shy away from anything that would make the user have to do anything else (for which they would need to read something that they already aren't :) ) |
See discussion.
I asked "do you think it's possible to reduce the volume of docker compose files? I was a bit perplexed by the relationship between rest-fights/deploy/docker-compose/native.yml and rest-fights/src/main/docker-compose/native.yml, for example. One seems to be generated and the other is hand-maintained? For the UI project they're basically identical, but for others there's overlap, but also different content. I couldn't figure out the intention from a search of the readmes. (And a readme probably isn't the right place for that kind of information, because people might not be able to find it mixed in with all the other content, and when they need is when they're looking at the file, trying to understand if it's ok to update or if it's generated.)"
@edeandrea answered:
"For the docker compose stuff, yes src/main are hand maintained "snippets" whereas in the deploy directory is the "real" docker compose file. It's done this way so that we can have the root /deploy/docker-compose with an uber compose file that had everything from all the apps in it."
"It was done that way to try and mimic the kubernetes extension. If we did away with that we'd have to hand craft 2 sets of compose files - 1 individually for each app and 1 uber compose file, both for jvm (and perhaps multiple jvm versions) and native."
The text was updated successfully, but these errors were encountered: