-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add umask as a global state argument #57803
Conversation
@terminalmage Looks good. My only comment is on the change in doc/ref/states/requisites.rst, why this is temporary? Also we need a changelog file added and the pre-commit test is failing. |
It's temporary because the umask is only changed for the lifetime of a single state. Perhaps the wording can be improved? |
27f8c8d
to
c2d5069
Compare
@terminalmage Thanks. That makes more sense. My first impression was that it was being changed for just the 3002 release and would change to a different approach in the future. |
@garethgreenaway Cool. Should this just be considered an "added" changelog entry even though |
Also, can the changelog files contain markdown? This is not clear from the docs. |
@terminalmage I would make it an "changed" change since you're remove the |
OK, thanks. Changelog file added. |
8ab0e30
to
6047e6e
Compare
@terminalmage mind resolving the merge conflicts? |
2207749
to
1c88fcc
Compare
This removes the umask argument from cmd states and makes it a global state argument.
1c88fcc
to
f308e92
Compare
Merge conflicts removed, tests re-done and passing |
The 3002 version number in the docs need to be changed to 3006: https://docs.saltproject.io/en/master/ref/states/requisites.html#run-state-with-a-different-umask |
This removes the umask argument from cmd states and makes it a global state argument.