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

Check dir_mode recursively in file.directory #53385

Merged
merged 8 commits into from Jun 6, 2019

Conversation

@Akm0d
Copy link
Contributor

commented Jun 5, 2019

What does this PR do?

Fixes a problem in states.file where directory permissions are not checked properly with test=true
Original fix by @amendlik here:
#52175

What issues does this PR fix or reference?

Partially fixes #52173

Tests written?

Yes

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Akm0d and others added 3 commits Jun 5, 2019
A change was introduced in 79f6b42 (PR #50653) to check the file
mode recursively. This change introduced a variable shadow that
caused directory permission checking to fail.
Akm0d added 2 commits Jun 5, 2019
This reverts commit 26fd01a.
tests/unit/states/test_file.py Outdated Show resolved Hide resolved
@twangboy twangboy changed the title Check dir_mode recursively in file.directory [WIP] Check dir_mode recursively in file.directory Jun 6, 2019
@saltstack saltstack deleted a comment from Akm0d Jun 6, 2019
@Akm0d Akm0d changed the title [WIP] Check dir_mode recursively in file.directory Check dir_mode recursively in file.directory Jun 6, 2019
Akm0d added 2 commits Jun 6, 2019
@Akm0d Akm0d requested a review from saltstack/team-core Jun 6, 2019
@cmcmarrow cmcmarrow self-requested a review Jun 6, 2019
@Akm0d Akm0d merged commit 477cd20 into saltstack:2019.2.1 Jun 6, 2019
17 checks passed
17 checks passed
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint Python lint test has passed
Details
jenkins/pr/py2-centos-6 The py2-centos-6 job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-debian-8 The py2-debian-8 job has passed
Details
jenkins/pr/py2-debian-9 The py2-debian-9 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-ubuntu-1804 The py2-ubuntu-1804 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-debian-8 The py3-debian-8 job has passed
Details
jenkins/pr/py3-debian-9 The py3-debian-9 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-ubuntu-1804 The py3-ubuntu-1804 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
@amendlik

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

@Akm0d, I appreciated your response over on #52211 about how to prevent discouraging community contributions. Another thing to avoid is squashing commits. When you do that, the entire change history is added to the repo as a single commit, with only yourself as the author. It means that the original authors do not get credit in the repository for the work they've done.

@Akm0d

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

@amendlik That is a valid point, I thought it looked cleaner to squash but hadn't considered all the implications. In the future I'll keep that in mind and preserve the history/credit due to authors and only squash my own commits. Sorry about that.

@Akm0d Akm0d referenced this pull request Jun 11, 2019
Akm0d added a commit that referenced this pull request Jun 14, 2019
Backport #53385 into Neon
@whytewolf

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

Is this going to be backported to 2018.3 that also has this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.