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

[FIX] skip bundler gem when warning when an explicitly updated spec d… #6155

Closed

Conversation

oded-zahavi
Copy link

…oes not get a newer version (618c09b)

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

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

The problem was when running bundle update --group xxx

What was your diagnosis of the problem?

My diagnosis was that bundler was taken into consideration and fails the updating process

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

My fix is to skip bundler when checking whether to warn when an explicitly updated spec does not get a newer version

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

I chose this fix because it is a rather new code (added in v1.16.0) and it is rather simple and doesn't require lots of changes in code. Another option would be to revert the lines added between 1.15.4 and 16.0.0 altogether. Up to the maintainers to decide.

@ghost
Copy link

ghost commented Nov 7, 2017

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

@oded-zahavi oded-zahavi force-pushed the fix_bundle_update_with_group branch from 95ea79b to e347299 Compare November 7, 2017 12:43
@colby-swandale
Copy link
Member

Thanks for opening a pull request, we will need a test for this change to make sure we don't regress in the future.

@bundlerbot
Copy link
Collaborator

☔ The latest upstream changes (presumably #6157) made this pull request unmergeable. Please resolve the merge conflicts.

@segiddins
Copy link
Member

Looks good with an added test case!

@oded-zahavi
Copy link
Author

Thanks @segiddins any plans to merge any time soon?

@colby-swandale
Copy link
Member

@oded-zahavi This still requires a test and this also needs to rebase with master before we merge.

@oded-zahavi
Copy link
Author

oded-zahavi commented Nov 17, 2017 via email

@segiddins
Copy link
Member

Yes, please add a test so we don't regress in the future

@colby-swandale
Copy link
Member

ping @oded-zahavi

@oded-zahavi oded-zahavi closed this Nov 9, 2018
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.

4 participants