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

Adds full_return support in libpepper. #175

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

RyanFrantz
Copy link

  • This allows folks using this as a library to get the same behavior as CLI (cli.run() sets full_return=True), if they so choose.

- This allows folks using this as a library to get the same behavior as CLI (cli.run() set full_return=True), if they so choose.
@RyanFrantz RyanFrantz closed this Oct 22, 2018
@RyanFrantz RyanFrantz reopened this Oct 22, 2018
@codecov
Copy link

codecov bot commented Oct 22, 2018

Codecov Report

Merging #175 into develop will decrease coverage by 0.72%.
The diff coverage is 12.5%.

@@             Coverage Diff             @@
##           develop     #175      +/-   ##
===========================================
- Coverage    72.39%   71.66%   -0.73%     
===========================================
  Files            5        5              
  Lines          594      600       +6     
  Branches       126      129       +3     
===========================================
  Hits           430      430              
- Misses         137      142       +5     
- Partials        27       28       +1

@gtmanfred
Copy link
Contributor

I am working on fixing the tests in travis, would you please add some tests for the extra branchs you added for full_return?

Copy link
Contributor

@gtmanfred gtmanfred left a comment

Choose a reason for hiding this comment

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

Can I get some tests to test this addition?

Thanks,
Daniel

@RyanFrantz
Copy link
Author

Coming back around to this. I'll poke this a bit next week, including syncing with upstream master and writing tests.

@gtmanfred
Copy link
Contributor

Awesome, thanks!

@nicholasmhughes
Copy link
Collaborator

Howdy! 👋

Apologies for the long delay on this integrating this code. We certainly appreciate your effort!

Pepper is getting some renewed focus and love, and we're checking in to see if you'd like to update this against the latest version of the base branch and ensure that all merge conditions are met.

If the items addressed by this contribution have already been fixed/added, are no longer an issue, or you no longer have a desire to work on this... please let us know and/or close out this PR.

If we don't hear from you within 7 days, we'll assume the code has been abandoned and close it out. If you'd like to revisit this in the future, you can always open a new PR.

Thank you, and we hope to see you active in the Salt ecosystem in the future!

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