Explicitly report that the installation command is not required

[NikolayRys]

If there's nothing to install, the bundler:install task is not going to show up in the console output or logs, which could create a moment of puzzlement to newer folks. Since it's not really possible to see in this case whether the gem is configured correctly or even operates at all.

I propose instead to show a one-line message to document that it goes as expected:

00:06 bundler:install
           The Gemfile's dependencies are satisfied, skipping installation

[capistrano-bot]

Thanks for the PR! This project lacks automated tests, which makes reviewing and approving PRs somewhat difficult. Please make sure that your contribution has not broken backwards compatibility or introduced any risky changes.

Generated by :no_entry_sign: Danger

[NikolayRys]

@mattbrictson if you could have a look, please

[will-in-wi]

Code-wise, this seems fine to me, although I think unless/else blocks tend to be confusing and prefer to switch them (although we have this turned off for a reason in Rubocop, so don't worry about it).

I don't really have much of an opinion on the change itself. I think it does improve UX.

[NikolayRys]

I can swap it for an if there, of course.

[NikolayRys]

Done, now it's reversed.

[NikolayRys]

@will-in-wi could you please also check this one?

[will-in-wi]

I added a comment about which I am uncertain, and tagged the other two maintainers for review. I'll do some research, and hopefully Matt or Lee can take a look at this. Once I get one more review, or I get some time to make sure my comment is accurate, I'll merge. This makes sense to me.

[leehambley]

"When bundle install is skipped due to a Gemfile's dependencies are being satisfied, print a message to the log instead of silently skipping"

Wow, thanks, I've looked at this PR a bunch of times and I couldn't figure out why it was skipping bundle install, I could see the call... Thanks for the context.

Sent from my phone from underway.

On Sat, 13 Oct 2018, 21:26 Matt Brictson, notifications@github.com wrote:

@mattbrictson commented on this pull request.

In CHANGELOG.md https://github.com/capistrano/bundler/pull/106#discussion_r224969702:

@@ -1,6 +1,7 @@

[Unreleased][] (master)

• Your contribution here! +* Explicitly reporting that the installation command is not required

Can you make this more descriptive? How about: "When bundle install is skipped due to a Gemfile's dependencies are being satisfied, print a message to the log instead of silently skipping"

In lib/capistrano/tasks/bundler.cap https://github.com/capistrano/bundler/pull/106#discussion_r224969727:

@@ -28,7 +28,9 @@ namespace :bundler do options = [] options << "--gemfile #{fetch(:bundle_gemfile)}" if fetch(:bundle_gemfile) options << "--path #{fetch(:bundle_path)}" if fetch(:bundle_path)

• unless test(:bundle, :check, *options)
• if test(:bundle, :check, *options)
• output.log("The Gemfile's dependencies are satisfied, skipping installation")

I'd prefer we use info to be consistent with how we output similar messages in capistrano-rails:

https://github.com/capistrano/rails/blob/0e14a506b63b17b7c2faeae4d083f569b0aa4920/lib/capistrano/tasks/migrations.rake#L11

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/capistrano/bundler/pull/106#pullrequestreview-164473828, or mute the thread https://github.com/notifications/unsubscribe-auth/AABKCJdukde6ZPvfcSxjokGpmxfqs8E1ks5ukj54gaJpZM4VjXq- .

[NikolayRys]

Adjusted