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

Make --dry-run flag consistent across rubygems commands #3867

Merged
merged 4 commits into from
Sep 3, 2020

Conversation

bronzdoc
Copy link
Member

Description:

closes #3866

Make --dryrun flag consistent across Rubygems commands


I will abide by the code of conduct.

@nobu
Copy link
Contributor

nobu commented Jul 31, 2020

GNU make also provides -n, --just-print, --dry-run, --recon.

@duckinator
Copy link
Member

@bronzdoc on master, --dryrun is only used once across Bundler and RubyGems, whereas --dry-run is used twice and seems to be the norm elsewhere. Given that, I'd rather see gem cleanup switched to use --dry-run with --dryrun as a backwards-compatible alias, because it would make the whole RubyGems+Bundler pair more internally consistent and more consistent with other software.

@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Aug 1, 2020

I agree with @duckinator!

@deivid-rodriguez
Copy link
Member

@bronzdoc Do you agree with @duckinator's suggestion? Are you planning to update the PR with that approach?

@bronzdoc
Copy link
Member Author

Dang, I totally forgot about this... I agree!

Will Work on Rubygems later today and will update this PR sorry for taking that long fellas

@bronzdoc bronzdoc changed the title Make --dryrun flag consistent across rubygems commands Make --dry-run flag consistent across rubygems commands Aug 31, 2020
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

I saw that the --update-sources option to gem install passes a :Deprecated first argument to add_option and the consequence of this is that the option appears under a separate group called Deprecated Options in gem install --help output. Maybe it's a good idea to do it here too?

@bronzdoc
Copy link
Member Author

Should I deprecate -d to and stick to just -n since im already touching those flags in this PR?

@deivid-rodriguez
Copy link
Member

Not sure 🤔. I think we can leave it as it is, and remove it if it confuses someone, but I don't have an issue with deprecating it either.

@bronzdoc
Copy link
Member Author

Ok, sounds good

Copy link
Member

@duckinator duckinator left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@deivid-rodriguez
Copy link
Member

CI failure is unrelated (possibly fixed by #3850), so merging.

Thanks @bronzdoc!

@deivid-rodriguez deivid-rodriguez merged commit 8bf3994 into master Sep 3, 2020
@deivid-rodriguez deivid-rodriguez deleted the fix-cleanup-flag-name branch September 3, 2020 09:45
@hsbt hsbt added this to the 3.2.0 milestone Sep 4, 2020
hsbt pushed a commit that referenced this pull request Sep 23, 2020
Make --dry-run flag consistent across rubygems commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dry-run options
6 participants