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

removing multiline gem specifications correctly#7454

Closed
jethroo wants to merge 1 commit intorubygems:masterfrom
jethroo:fix/Issue_7431_remove_multi_line_gems_correctly
Closed

removing multiline gem specifications correctly#7454
jethroo wants to merge 1 commit intorubygems:masterfrom
jethroo:fix/Issue_7431_remove_multi_line_gems_correctly

Conversation

@jethroo
Copy link
Copy Markdown

@jethroo jethroo commented Nov 26, 2019

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

This PR fixes the issue #7431 where gems specified in multiple lines were not removed correctly.

What was your diagnosis of the problem?

The remove was implemented in a way that the match was only done for a line each.

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

Now upon line removal it is checked if the gem specification was finished in this line. If there is a trailing , comma further lines needs to be removed.

@welcome
Copy link
Copy Markdown

welcome bot commented Nov 26, 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

@jethroo jethroo force-pushed the fix/Issue_7431_remove_multi_line_gems_correctly branch from 8fe7fa5 to 142c0dd Compare November 26, 2019 15:45
gemfile <<-G
source '#{file_uri_for(gem_repo1)}'

gem 'rack',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the extra gems that are the original report?

Like this:

gem 'git'
gem 'some-gem',
    git: 'ssh://gitlab-server.com/project/repo.git',
    branch: 'master'
gem 'nokogiri'

And add expectation for git and nokogiri.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the feedback, sure would give more test context, will do soonish ;)

@jethroo jethroo force-pushed the fix/Issue_7431_remove_multi_line_gems_correctly branch from 142c0dd to c17351d Compare December 10, 2019 13:02
@jethroo jethroo requested a review from hsbt December 10, 2019 14:45
@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Haven't reviewed this, but I agree with fixing this 👍.

However, this kind of maintenance burden reenforces my position that we should not accept features like #6623 or #6597.

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.

3 participants