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

saltcheck module #43065

Merged
merged 38 commits into from Sep 1, 2017
Merged

saltcheck module #43065

merged 38 commits into from Sep 1, 2017

Conversation

wcannon
Copy link
Contributor

@wcannon wcannon commented Aug 18, 2017

What does this PR do?

This pull request adds a salt module for salt state logic checking

What issues does this PR fix or reference?

none

New Behavior

Remove this section if not relevant
Allows for unit-test-like testing of salt states and highstates

Tests written?

Yes

Please review Salt's Contributing Guide for best practices.

tsa-devops added 30 commits Jul 20, 2017
@cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 21, 2017

I do like this quite a lot but I wouldn't feel comfortable getting it in until there is more substantial documentation in the Salt docs themselves, versus just having it in the module.

@gtmanfred What do you think about perhaps creating a Testing tutorial that talks about both this approach and the new salt-kitchen stuff?

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Aug 21, 2017

I think that would be great!

@wcannon
Copy link
Contributor Author

@wcannon wcannon commented Aug 22, 2017

@cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 23, 2017

@wcannon or @gtmanfred Any volunteers to start on a tutorial? :] I'm happy to merge this before then but it would be great if we could get started ASAP so it's not forgotten.

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Aug 23, 2017

I will add it to my list.

@wcannon
Copy link
Contributor Author

@wcannon wcannon commented Aug 25, 2017

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Aug 25, 2017

@wcannon
Copy link
Contributor Author

@wcannon wcannon commented Aug 25, 2017

@cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 25, 2017

@gtmanfred How do you want to handle this. Do you want to just submit a PR to @wcannon 's branch or do something independently?

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Aug 25, 2017

Can we merge this PR, and then I will start on the documentation for this and the tutorial for testing next week?

Thanks,
Daniel

@cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 28, 2017

@gtmanfred Yes, I have no problem with that at all. It does look like it failed a lint check though. I'll re-run the test and then hopefully @wcannon can fix it right up.

@cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 28, 2017

re-run lint

gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented on a3bbe16 Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, you should be able to use __utils__['files.fopen'] and not import this.

but this is fine

@wcannon
Copy link
Contributor Author

@wcannon wcannon commented Aug 31, 2017

Anything else left to do?

@wcannon wcannon closed this Aug 31, 2017
@gtmanfred gtmanfred reopened this Aug 31, 2017
@rallytime rallytime merged commit 390ea9e into saltstack:develop Sep 1, 2017
6 checks passed
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.

None yet

5 participants