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

Remove if branch that always evaluate to false#6716

Closed
gabrielgiordano wants to merge 1 commit intorubygems:masterfrom
gabrielgiordano:remove-impossible-if-branch
Closed

Remove if branch that always evaluate to false#6716
gabrielgiordano wants to merge 1 commit intorubygems:masterfrom
gabrielgiordano:remove-impossible-if-branch

Conversation

@gabrielgiordano
Copy link
Copy Markdown

loaded_spec = nil always evaluate to false, so the code in this if is never executed.

I'm not sure if this is a bug or if the branch is not necessary. Since I don't know the codebase well enough, I'm opening the pull request to at least bring the attention to a possible problem.

`loaded_spec = nil` always evaluate to false, so the code in this if
is never executed.
@ghost
Copy link
Copy Markdown

ghost commented Sep 29, 2018

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

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Thanks @gabrielgiordano. This is the alternative to #6687. I'm not sure what's best, but I think I lean torwards this alternative?

@colby-swandale
Copy link
Copy Markdown
Member

I agree, we should be fixing the clause rather than just removing it.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

He, I actually meant that I think I prefer this PR, since we wouldn't change how bundler is currently working in any way... But I'm not really sure...

@colby-swandale
Copy link
Copy Markdown
Member

colby-swandale commented Sep 30, 2018

Ah, sorry i didn't read that correctly. But i still think we should fix it rather then just remove it.

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