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

Make 'bundle add' cache newly added gems when needed #7393

Merged
1 commit merged into from Nov 6, 2019

Conversation

andrehjr
Copy link
Contributor

@andrehjr andrehjr commented Oct 25, 2019

What was the end-user problem that led to this PR?

The problem was that when calling bundle add with cache_all as true, I have to call an additional bundle install to vendor gems.

What was your diagnosis of the problem?

My diagnosis was that a call to Bundler.load.cache was missing. For example Bundler::CLI::Update does the same after installing gems.

What is your fix for the problem, implemented in this PR?

My fix was to call Bundler.load.cache when Bundler.app_cache.exist?

Why did you choose this fix out of the possible options?

I chose this fix because it looks like this makes the behavior consistent with other commands.

I should also say that, as this is my first PR here, I'm not sure that this is the best solution, and it seems an easy fix for #7384.

@welcome
Copy link

welcome bot commented Oct 25, 2019

Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack.

For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide

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.

Thanks @andrehjr for your contribution!

Fix looks good to me in general, just added a few comments.

lib/bundler/cli/add.rb Outdated Show resolved Hide resolved
spec/commands/add_spec.rb Outdated Show resolved Hide resolved
spec/commands/add_spec.rb Outdated Show resolved Hide resolved
@andrehjr andrehjr changed the title Calling bundle add with cache_all as true, it caches the newly added gems Make 'bundle add' cache newly added gems when needed Nov 4, 2019
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.

Thanks! It looks great!

@deivid-rodriguez
Copy link
Member

@bundlerbot merge

ghost pushed a commit that referenced this pull request Nov 6, 2019
7393: Make 'bundle add' cache newly added gems when needed r=deivid-rodriguez a=andrehjr

### What was the end-user problem that led to this PR?

The problem was that when calling `bundle add` with cache_all as true, I have to call an additional `bundle install` to vendor gems.

### What was your diagnosis of the problem?

My diagnosis was that a call to Bundler.load.cache was missing. For example Bundler::CLI::Update does the same after installing gems.

### What is your fix for the problem, implemented in this PR?

My fix was to call Bundler.load.cache when `Bundler.app_cache.exist? ` 

### Why did you choose this fix out of the possible options?

I chose this fix because it looks like this makes the behavior consistent with other commands.

I should also say that, as this is my first PR here, I'm not sure that this is the best solution, and it seems an easy fix for #7384. 

Co-authored-by: André Luis Leal Cardoso Junior <andrehjr@gmail.com>
@ghost
Copy link

ghost commented Nov 6, 2019

Build succeeded

@ghost ghost merged commit 293ad80 into rubygems:master Nov 6, 2019
@deivid-rodriguez deivid-rodriguez added this to the 2.1.0.pre.3 milestone Nov 6, 2019
deivid-rodriguez pushed a commit that referenced this pull request Nov 7, 2019
7393: Make 'bundle add' cache newly added gems when needed r=deivid-rodriguez a=andrehjr

### What was the end-user problem that led to this PR?

The problem was that when calling `bundle add` with cache_all as true, I have to call an additional `bundle install` to vendor gems.

### What was your diagnosis of the problem?

My diagnosis was that a call to Bundler.load.cache was missing. For example Bundler::CLI::Update does the same after installing gems.

### What is your fix for the problem, implemented in this PR?

My fix was to call Bundler.load.cache when `Bundler.app_cache.exist? `

### Why did you choose this fix out of the possible options?

I chose this fix because it looks like this makes the behavior consistent with other commands.

I should also say that, as this is my first PR here, I'm not sure that this is the best solution, and it seems an easy fix for #7384.

Co-authored-by: André Luis Leal Cardoso Junior <andrehjr@gmail.com>
(cherry picked from commit f83412a)
hsbt added a commit to ruby/ruby that referenced this pull request Nov 11, 2019
  Features:
    - Add caller information to some deprecation messages to make them easier to fix [#7361](rubygems/bundler#7361)
    - Reconcile `bundle cache` vs `bundle package` everywhere. Now in docs, CLI help and everywhere else `bundle cache` is the preferred version and `bundle package` remains as an alias [#7389](rubygems/bundler#7389)
    - Display some basic `bundler` documentation together with ruby's RDoc based documentation [#7394](rubygems/bundler#7394)

  Bugfixes:
    - Fix typos deprecation message and upgrading docs [#7374](rubygems/bundler#7374)
    - Deprecation warnings about `taint` usage on ruby 2.7 [#7385](rubygems/bundler#7385)
    - Fix `--help` flag not correctly delegating to `man` when used with command aliases [#7388](rubygems/bundler#7388)
    - `bundle add` should cache newly added gems if an application cache exists [#7393](rubygems/bundler#7393)
    - Stop using an insecure folder as a "fallback home" when user home is not defined [#7416](rubygems/bundler#7416)
    - Fix `bundler/inline` warning about `Bundler.root` redefinition [#7417](rubygems/bundler#7417)
@andrehjr andrehjr deleted the make-bundle-add-vendorized branch January 6, 2020 01:06
This pull request was closed.
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.

None yet

2 participants