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 in support for reserved_percentage kwarg and correct documentation. #54500

Closed

Conversation

qcpeter
Copy link

@qcpeter qcpeter commented Sep 16, 2019

What does this PR do?

What issues does this PR fix or reference?

#54426

Previous Behavior

Misleading documentation.

New Behavior

Adds in reserved_percentage kwarg and updates documentation.

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

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

@qcpeter qcpeter requested a review from a team as a code owner September 16, 2019 14:23
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

any chance we can get a test added here to ensure this does not regress? There are currently some unit tests here ./tests/unit/modules/test_extfs.py Other than that this looks great, nice catch :)

@qcpeter
Copy link
Author

qcpeter commented Sep 19, 2019

I'm afraid I'm going away for ten days so will be unable to take a look at this immediately. Presumably you're thinking of some sort of test that passes in the kwargs and tests the flags that are passed the cmd.run mock? The existing tests are pretty minimal.

@waynew
Copy link
Contributor

waynew commented Oct 1, 2019

@qcpeter yeah, that should be sufficient. This doesn't really have a lot of logic, so it shouldn't be too difficult. There may be a test that already exists that could just be extended, but if not, what you described should be 👍

@dwoz
Copy link
Contributor

dwoz commented Dec 14, 2019

@qcpeter This needs to be re-opened against the master branch. We are no longer
accepting PRs to 2018.3. You can link back to this PR to preserve the
discussion. Sorry taking so long to get to this and for any confusion.

@dwoz dwoz closed this Dec 14, 2019
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.

4 participants