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.

Sign up for GitHub

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

Jump to bottom

Fix gem update --user-install #2901

Merged
4 commits merged into from
Oct 31, 2019
Merged

Fix gem update --user-install #2901

4 commits merged into from
Oct 31, 2019

Conversation

bronzdoc
Copy link
Member

Description:

Closes #2886

Update only user insatalled gems when the --user-install flag is passed to gem update

I will abide by the code of conduct.

@bronzdoc bronzdoc removed the wip label Oct 26, 2019
@duckinator
Copy link
Member

@bronzdoc should this perhaps print a message with a quick explanation that non-user gems are being skipped + how to update those?

Other than that, this looks good to me overall, but would definitely be a breaking change so I'm not sure when it should be merged.

@bronzdoc
Copy link
Member Author

bronzdoc commented Oct 26, 2019 •
edited
Loading

@duckinator I don't know 🤔 gem update --user-install updating system-wide gems seems like a bug to me.

@MSP-Greg
Copy link
Contributor

I agree with both of you. But, I'd prefer the change. I'm always updating my local ruby-head build, so all my gems used for testing are in --user-install...

@duckinator
Copy link
Member

@bronzdoc thinking on it a bit more, I can see arguments in either direction. I'm cool with it being merged and treated as a bugfix if other folks are. 👍

@deivid-rodriguez
Copy link
Member

I'll review this tomorrow, 👍. I do agree in principle with considering this a bug.

deivid-rodriguez
deivid-rodriguez requested changes Oct 29, 2019
View reviewed changes
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.

The test and the fix look great. There's just a bunch of changes in lib/rubygems/specification.rb that I don't think we need.

test/rubygems/test_gem_commands_update_command.rb Show resolved Hide resolved
lib/rubygems/commands/update_command.rb Show resolved Hide resolved
@bronzdoc
Copy link
Member Author

@bundlerbot try

@bronzdoc
Copy link
Member Author

Build is not running after a force push 🙄

@deivid-rodriguez
Copy link
Member

Let's see how our bot is feeling now :)

@bundlerbot merge

ghost pushed a commit that referenced this pull request Oct 31, 2019
@bundlerbot @bronzdoc
Merge #2901
a2bd981 
2901: Fix gem update --user-install r=deivid-rodriguez a=bronzdoc

# Description:

Closes #2886

Update only user insatalled gems when the `--user-install` flag is passed to `gem update`

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: bronzdoc <lsagastume1990@gmail.com>
@deivid-rodriguez deivid-rodriguez mentioned this pull request Oct 31, 2019
Closed
@ghost
Copy link

ghost commented Oct 31, 2019

Build succeeded

  • macos (2.3.x)
  • macos (2.4.x)
  • macos (2.5.x)
  • macos (2.6.x)
  • ubuntu (2.3.x, bundler)
  • ubuntu (2.3.x, rubygems)
  • ubuntu (2.4.x, bundler)
  • ubuntu (2.4.x, rubygems)
  • ubuntu (2.5.x, bundler)
  • ubuntu (2.5.x, rubygems)
  • ubuntu (2.6.x, bundler)
  • ubuntu (2.6.x, rubygems)
  • ubuntu_bundler_master (2.6.x)
  • ubuntu_lint
  • ubuntu_rvm (jruby-9.2.8.0, rubygems)
  • ubuntu_rvm (ruby-head, bundler)
  • ubuntu_rvm (ruby-head, rubygems)
  • windows (2.4.x)
  • windows (2.5.x)
  • windows (2.6.x)

@ghost ghost merged commit 50d1251 into master Oct 31, 2019
@deivid-rodriguez deivid-rodriguez deleted the fix-gem-update branch October 31, 2019 10:16
@deivid-rodriguez deivid-rodriguez mentioned this pull request Oct 31, 2019
Open
5 tasks
@Irakaj93 Irakaj93 mentioned this pull request Dec 26, 2020
Closed
@voxik voxik mentioned this pull request Mar 3, 2023
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deivid-rodriguez deivid-rodriguez deivid-rodriguez requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gem update installs system-wide updates in home
4 participants
@bronzdoc @duckinator @MSP-Greg @deivid-rodriguez