Skip to content
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

added dir_mode logic for users.user_files #205

Merged
merged 2 commits into from
Aug 1, 2019

Conversation

sevrob
Copy link
Contributor

@sevrob sevrob commented Jul 30, 2019

Added logic for setting directory mode logic in users.user_files.

@aboe76 aboe76 requested a review from myii July 30, 2019 13:54
@myii
Copy link
Member

myii commented Jul 31, 2019

@sevrob Thanks for this PR, I'm fine with it in general.

@aboe76 We need to consider something about dir_mode: it's not supported on Windows. So how about a little comment in the pillar.example to explain that (just above the line added)? Ultimately, this isn't such a big problem because all the user has to do is not add this to their pillar.

However, I checked within this formula and there is a dir_mode problem, where it is being used without the option to unset it:

- dir_mode: 700

This one will need to be resolved, to prevent issues for Windows minions. I don't have any to hand to see what error is displayed but we shouldn't do this in any case.

@sevrob
Copy link
Contributor Author

sevrob commented Jul 31, 2019

new comment to pillar.example added.

@aboe76
Copy link
Member

aboe76 commented Jul 31, 2019

@sevrob can you add the fix for the dir mode here?

- dir_mode: 700

after that I'm fine with merging

@myii myii merged commit 83ffcb4 into saltstack-formulas:master Aug 1, 2019
@myii
Copy link
Member

myii commented Aug 1, 2019

@aboe76 I've just merged this because this PR is complete for what it does and I don't want to trouble @sevrob further for something that is unrelated to this PR's changeset. I've captured this issue under #206 instead.

Thanks for this PR, @sevrob. If you do want to help out with #206, that would be appreciated.

@aboe76
Copy link
Member

aboe76 commented Aug 1, 2019

@myii good call, @sevrob thanks for you contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants