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

Fix multiple gem files deprecation#6999

Merged
3 commits merged intomasterfrom
multiple_gemfiles_deprecation
Mar 12, 2019
Merged

Fix multiple gem files deprecation#6999
3 commits merged intomasterfrom
multiple_gemfiles_deprecation

Conversation

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez commented Mar 1, 2019

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

The problem was the current message about deprecating Gemfile in favor of gems.rb was incorrect, and this I'm not sure when it was supposed to be displayed. Its corresponding test was also failing.

What was your diagnosis of the problem?

My diagnosis was that the intention was not to deprecate Gemfile's for the time being, but only warn if the user opts in to gems.rb but does not remove the old Gemfile.

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

My fix is to properly detect the situation where both Gemfile and gems.rb files are present, and show proper messages guiding the user to properly make the switch.

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

I chose this fix because I think it works and doesn't take this migration too far, since the ecosystem is not yet ready for this at all.

The previous logic is unclear to me. It seemed to try to detect only
"multiple gemfile situations", but it was doing it incorrectly, I think.

The new message is printed _only_ if both gems.rb and Gemfile are
detected in the same project. And recommends sticking with gems.rb. But
we are not yet deprecating "Gemfile" other than that.
@deivid-rodriguez deivid-rodriguez changed the title Multiple gemfiles deprecation Fix multiple gemfiles deprecation Mar 1, 2019
@deivid-rodriguez deivid-rodriguez changed the title Fix multiple gemfiles deprecation Fix multiple gem files deprecation Mar 1, 2019
@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

I'm doing this for the sake of moving forward, but feel free to leave feedback after merge and I'll change anything that's needed!

@bundlerbot r+

ghost pushed a commit that referenced this pull request Mar 12, 2019
6999: Fix multiple gem files deprecation r=deivid-rodriguez a=deivid-rodriguez

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

The problem was the current message about deprecating `Gemfile` in favor of `gems.rb` was incorrect, and this I'm not sure when it was supposed to be displayed. Its corresponding test was also failing.

### What was your diagnosis of the problem?

My diagnosis was that the intention was not to deprecate Gemfile's for the time being, but only warn if the user opts in to `gems.rb` but does not remove the old `Gemfile`.

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

My fix is to properly detect the situation where both `Gemfile` and `gems.rb` files are present, and show proper messages guiding the user to properly make the switch.

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

I chose this fix because I think it works and doesn't take this migration too far, since the ecosystem is not yet ready for this at all.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@ghost
Copy link
Copy Markdown

ghost commented Mar 12, 2019

Build succeeded

@ghost ghost merged commit c1df6bc into master Mar 12, 2019
@ghost ghost deleted the multiple_gemfiles_deprecation branch March 12, 2019 09:11
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.

1 participant