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

Add file.tidied similar to Puppet's tidy resource. #47718

Merged
merged 4 commits into from Aug 5, 2018
Merged

Add file.tidied similar to Puppet's tidy resource. #47718

merged 4 commits into from Aug 5, 2018

Conversation

@dhs-rec
Copy link
Contributor

@dhs-rec dhs-rec commented May 18, 2018

What does this PR do?

Adds file.tidied as inspired by Puppet

What issues does this PR fix or reference?

Implements #46752

Tests written?

No

Commits signed with GPG?

No

@cachedout
Copy link
Contributor

@cachedout cachedout commented May 18, 2018

I left some comments in #46752 on this.

# Helper to match a given name against one or more glob patterns
def _matches(name, patterns=[]):
for pattern in patterns:
if fnmatch.fnmatch(name, pattern):

This comment has been minimized.

@gtmanfred

gtmanfred May 20, 2018
Contributor

I would use regex instead of fnmatch

This comment has been minimized.

@dhs-rec

dhs-rec May 22, 2018
Author Contributor

Done.

@rallytime
Copy link
Contributor

@rallytime rallytime commented May 22, 2018

@dhs-rec Can you write some tests for this new function?

@dhs-rec
Copy link
Contributor Author

@dhs-rec dhs-rec commented May 24, 2018

Once I manage to wrap my mind around the test stuff...

@rallytime
Copy link
Contributor

@rallytime rallytime commented May 24, 2018

@dhs-rec Awesome! Thank you!

There's some docs on testing here, if it helps.

@rallytime
Copy link
Contributor

@rallytime rallytime commented Jul 3, 2018

Hi @dhs-rec - just a bump on this one. Were you able to write some tests for this?

@dhs-rec
Copy link
Contributor Author

@dhs-rec dhs-rec commented Jul 9, 2018

No, sorry. Tried last week but I somehow didn't get it (I'm not that experienced in Python hacking). So I would be glad if someone else could jump in...

@rallytime
Copy link
Contributor

@rallytime rallytime commented Jul 27, 2018

@gtmanfred Do you think you could help out with some tests on this?

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Jul 27, 2018

Yeah, make an issue and i will work on them 👍

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Jul 30, 2018

@dhs-rec i have opened a PR on your branch, once you merge it we should be able to get this PR in.

https://github.com/dhs-rec/salt/pull/1

Thanks,
Daniel

@rallytime rallytime merged commit a2a5876 into saltstack:develop Aug 5, 2018
6 of 9 checks passed
6 of 9 checks passed
continuous-integration/jenkins/pr-merge This commit cannot be built
Details
codeclimate 5 issues to fix
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has failed
Details
@wip
WIP ready for review
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
@dhs-rec
Copy link
Contributor Author

@dhs-rec dhs-rec commented Aug 10, 2018

@gtmanfred: Great, thanks a lot!

@dhs-rec dhs-rec deleted the dhs-rec:add_file_tidied branch Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants