-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Remove Comments from Configuration files #137
Comments
Discussed a bit with Max getting rid of Sandbox settings file entirely and using role vars. |
As long as docs help point the user to how the inventory works for must set settings, sure. But it does make it slightly more complicated to configure apps where settings change over time etc as we cannot migrate the settings. |
I do think we should think this through as making stuff harder just to avoid managing a config file isn't a good reason. |
This should be looked at with the traefik 3 branch before we push it out. I think we need to ferret through and find roles that need addressing so we have a list. |
Well, if we go with @owine's suggestion of ditching the config entirely then that solves this particular problem at least. However, it turns this into a docs problem that, in my opinion, would need to be sorted out before we release traefik3. |
I think docs might be a high priority item as well at this point - I don't think @RaneyDazed got too far with the list of docs needed from sandbox and there has been more added. The number of very basic questions I am seeing on discord for some apps makes me think this might be necessary too, recently have noticed a few autoscan questions in particular. |
What if we make sure all the current roles work fine and then go through all the docs for the roles and then make docs being ready in a PR/merged is required for any new roles or major changes that would require new information in the docs? I know I for one am a sinner in this regard but if we make it into a rule that everyone follows then maybe we'll have more luck enforcing good docs. |
I'd wanted to add more to the docs but I wasn't sure how much info was intended to be added to docs. Plus things just started getting a bit nuts with the list. I'm happy to start on them again. Just lmk exactly what you guys want added lol. I can't remember what exactly turned me off on doing all those docs but. Something got obnoxious. I need to get back to sarotate I think it was as well. |
I think there is a docs issue where I made templates but did not link them into the menu and listed them all. |
What do you mean by templates? Oh you added "docs" but didn't fill them in essentially? I'd seen that. I can try to get a few done tonight, but I have a ton of homework 😅 I'll do what I can if this is coming back around. :) |
Not asking you to do them ASAP, just mentioning that they are there and planned and incomplete. There are many other people who could help with this too. Yeh - I made an entry for each so it would be in the right place, already started with a basic guideline and should have been relatively easy for someone to flesh out. I think I might need to make another pass and see what roles have since been added with no docs too. |
I'm sure theres a few new ones but I don't think there's that many. Oh one of the things that was keeping me from doing more was git being hard to fully understand lmao. |
Yeh - ideally we sort you and git out sooner rather than later. Sorry. |
Haha that would be great but no rush. I can get it going as long as there's no commits in between my working on it and the PR being approved :p |
Status on this? |
Just seen this issue. Is it worth having all the apps docs as an issue so people are aware of what needs to be updated. Or at the very least highlight it in this issue? |
The settings file has been cleaned out, but there may be still comments left in some of the role files themselves. |
Explanations need to live in our docs as the comments are removed when configuration files are upgraded. So please transition away from this and clean up the ones currently present when added to their respective pages on our docs.
The text was updated successfully, but these errors were encountered: