search

capistrano / bundler

Bundler support for Capistrano 3.x
MIT License
219 stars 83 forks source link

Switch to environment variables for defining bundler parameters #121

Closed will-in-wi closed 4 years ago

will-in-wi commented 4 years ago

Another approach to https://github.com/capistrano/bundler/issues/115

According to the documentation, Bundler can accept these parameters using environment variables. This PR switches the default parameter passing to these variables.

Turns into:

  INFO [5d242bfd] Running $HOME/.rbenv/bin/rbenv exec bundle install --jobs 4 --quiet as myuser@example.com
 DEBUG [5d242bfd] Command: cd /home/myuser/myapp/current && ( export RBENV_ROOT="$HOME/.rbenv" RBENV_VERSION="2.7.1" BUNDLE_DEPLOYMENT="true" BUNDLE_PATH="/var/www/shared/bundle" BUNDLE_WITHOUT="development test" ; $HOME/.rbenv/bin/rbenv exec bundle install --jobs 4 --quiet )

This will likely require a major version bump, unless we can think through all of the edge cases here.

capistrano-bot commented 4 years ago

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

leehambley commented 4 years ago

At first glance @will-in-wi it looks good. I think I concur about the major version bump, although I dare say people are so accustomed to Bundler randomly moving arguments around, and generally breaking the way things work, we don't have to be so cautious.

Personal preference speaking here, I like having the critical options printed in the output. It serves as some kind of documentation about what we expect as the "important" set of Bundler options (which seem to grow more and more numerous every day). I also subscribe to a somewhat outdated school of thought, that when scripting and writing automations one should always prefer explicit over implicit, so use the long form options, specify the argument even if it usually comes with a default, etc, etc.

I think if I parse between the lines in this PR that it's implied that people are customizing bundlers options in a way that means it's tricky to know what to set "the capistrano way", and which to set as the env?

Either way, if I can help you with any kinds of manual testing, just direct me :) I have access to a Ruby project which is still deployed with Capistrano using bundler, and willing colleagues in the devops department who'd be happy to perform some manual testing.

will-in-wi commented 4 years ago

The main benefit of this over the alternatives proposed is simply that we aren't depending on files being written to disk, and we aren't running multiple commands.

Within that, I'm fine with any level of printing.

I think in terms of testing, we mostly just need people to check that it works with their configuration.

will-in-wi commented 4 years ago

Ah… So the file actually has to be there for normal execution.

Well, so much for this. 😄 Thanks!