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 --bare behavior to sync and clean #3064

Closed
wants to merge 5 commits into from
Closed

Conversation

jcrotts
Copy link
Contributor

@jcrotts jcrotts commented Oct 19, 2018

Implement some behavior for --bare in sync and clean, and add bare an option to do_clean in cli.py

The issue

#3041

The fix

Added logic for --bare to sync and clean since they are passed in as arguments.
Excluding error messages.

Also added --bare as a click option to clean.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@uranusjr
Copy link
Member

I only see it be added to clean? Also, a news fragment to describe the change would be nice.

@uranusjr uranusjr added the PR: awaiting-news The PR related to this issue is missing a news fragment and cannot be merged. label Oct 19, 2018
@jcrotts
Copy link
Contributor Author

jcrotts commented Oct 19, 2018

--bare was an option for sync already in command.py, but I don't think it actually did anything.

Sure thing I'll add a fragment.

@uranusjr
Copy link
Member

I see. Does the sync command’s --bare option have the intended behaviour now? Since this PR does not change sync, it might be better to only mention adding --bare to clean in the commit message and news fragment.

@jcrotts
Copy link
Contributor Author

jcrotts commented Oct 19, 2018

One change was made to sync that removes one output line (sync doesn't have much output either way). I can change the commit message and news fragment if you think it would be clearer.

@uranusjr
Copy link
Member

It would be best to either 1) remove mention of sync from fragment, or 2) use sync’s bare option to reduce output. I am fine with either, your call. (And if you opt for 1), another PR on sync would be nice as well!)

@jcrotts
Copy link
Contributor Author

jcrotts commented Oct 19, 2018

Great I'll change the fragment to:

--bare now has an effect on clean, and sync's bare option is now used to reduce output.

Should I squash commits as well?

@uranusjr
Copy link
Member

You don’t need to, I can squash on merge (but you can squash yourself as well, there’s no difference).

@uranusjr uranusjr removed the PR: awaiting-news The PR related to this issue is missing a news fragment and cannot be merged. label Oct 19, 2018
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