Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

bundler bin for those who confuse it with bundle #2598

Merged
merged 1 commit into from
Aug 16, 2013

Conversation

kirs
Copy link
Contributor

@kirs kirs commented Aug 14, 2013

As mentioned in rubygems/bundler-features#12, we could provide an executable named "bundler" that prints a warning and silently execs to bundle.

ui = Bundler::UI::Shell.new
ui.error "It's recommended to use Bundler through 'bundle' binary instead 'bundler'"

Bundler.with_friendly_errors { Bundler::CLI.start }
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still a lot of duplication here. Any way to remove it? If you just exec you probably don't need the check old versions and thor debug stuff.

@xaviershay
Copy link
Contributor

Spec?

@kirs
Copy link
Contributor Author

kirs commented Aug 15, 2013

@xaviershay updated. I didn't find any specs for bin/bundle or bin/bundle_ruby executables and I thought there are not necessary. Are they?

ui.error "It's recommended to use Bundler through 'bundle' binary instead of 'bundler'"

bin = "#{File.dirname(__FILE__)}/bundle #{ARGV.join(" ")}"
exec "ruby #{bin}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the ruby here is redundant? bin/bundle is executable anyways. LGTM otherwise.

@xaviershay
Copy link
Contributor

Re specs, it's probably fine.

@kirs
Copy link
Contributor Author

kirs commented Aug 16, 2013

@xaviershay fixed. Thanks for the review!

xaviershay added a commit that referenced this pull request Aug 16, 2013
`bundler` bin for those who confuse it with `bundle`
@xaviershay xaviershay merged commit 37d929c into rubygems:master Aug 16, 2013
@xaviershay
Copy link
Contributor

Merged, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants