-
Notifications
You must be signed in to change notification settings - Fork 420
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
refactor(sls): make states more readable by spliting into sub-components #529
base: master
Are you sure you want to change the base?
Conversation
Add a verifier in each sls to exclude windows platform.
This permit to not break user states which depends on this one.
One point, the windows tests are failing because the
logs of
|
If (I didn't read the PR itself for now) |
Thanks @daks, that's exactly the kind of feedback I want. So, as an example, you prefer the following execution output:
over this one:
|
Yes, it seems a better solution. I don't use Windows but if I did I wouldn't want to have an error when everything is fine. And I consider that everything is fine because salt-master is just not supported on Windows. Of course, another solution would be to just run the compatible states. |
Ok, I consider an error to attribute an sls to an incompatible minion. But having errors that states can't be run could be an issue, I'll look at our it render with state warning instead of simple notification. |
You're not wrong. As I said, for my use case, I would simply not run the meta-state on Windows. Let see what other people think about that. I have no strong position on this. |
I think the error is better than the success message. not everyone knows windows is not supported for master. As a user, if I write |
PR progress checklist (to be filled in by reviewers)
What type of PR is this?
Primary type
[build]
Changes related to the build system[chore]
Changes to the build process or auxiliary tools and libraries such as documentation generation[ci]
Changes to the continuous integration configuration[feat]
A new feature[fix]
A bug fix[perf]
A code change that improves performance[refactor]
A code change that neither fixes a bug nor adds a feature[revert]
A change used to revert a previous commit[style]
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)Secondary type
[docs]
Documentation changes[test]
Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE
?No.
Related issues and/or pull requests
Describe the changes you're proposing
The current salt-formula is quite hard to read with lots of jinja conditionnals in the middle of states which is against the current standards.
This PR introduces a refactoring of the states into subcomponents:
master
TODO
and provides backward compatible state IDs which only notify user that their references must be changed.
Pillar / config required to test the proposed changes
Debug log showing how the proposed changes work
Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context