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

[Definition] Remove no-op code in expanded_deps#5387

Closed
chrismo wants to merge 1 commit intorubygems:masterfrom
chrismo:no_op_code_in_expanded_dependencies
Closed

[Definition] Remove no-op code in expanded_deps#5387
chrismo wants to merge 1 commit intorubygems:masterfrom
chrismo:no_op_code_in_expanded_dependencies

Conversation

@chrismo
Copy link
Copy Markdown
Contributor

@chrismo chrismo commented Jan 31, 2017

While troubleshooting my bundler-fixture gem, which hacks up a
Definition class with a fake Index, and needed updating for 1.14
support for ruby and RubyGems version in the Definition index, I
noticed this block of code appears to not do anything, since a 2nd
argument is not being passed into concat_ruby_version_requirements
method.

The specs that were altered in the same commit that added these lines
pass with or without the lines of code, so I'm committing this up to do
a full Travis run to see if there are any other consequences. If not,
then we either can remove these lines, or need to hook them up properly
and add proper failing specs.

While troubleshooting my bundler-fixture gem, which hacks up a
Definition class with a fake Index, and needed updating for 1.14
support for ruby and RubyGems version in the Definition index, I
noticed this block of code appears to not do anything, since a 2nd
argument is not being passed into `concat_ruby_version_requirements`
method.

The specs that were altered in the same commit that added these lines
pass with or without the lines of code, so I'm committing this up to do
a full Travis run to see if there are any other consequences. If not,
then we either can remove these lines, or need to hook them up properly
and add proper failing specs.
@chrismo
Copy link
Copy Markdown
Contributor Author

chrismo commented Feb 1, 2017

@segiddins dunno if i'll get time soon to dive into this, to figure out if this code should stay and be connected properly, and what the failing spec would be for it - or if this code can just be removed.

@segiddins
Copy link
Copy Markdown
Contributor

Let's rip it out for now

@segiddins
Copy link
Copy Markdown
Contributor

Or change it to concat_ruby_versions(version, ruby_versions) which is what I think it should be

@segiddins
Copy link
Copy Markdown
Contributor

@chrismo which approach would you prefer?

@segiddins
Copy link
Copy Markdown
Contributor

@chrismo ping

@bundlerbot
Copy link
Copy Markdown
Collaborator

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

@colby-swandale
Copy link
Copy Markdown
Member

ping @chrismo

@esasse
Copy link
Copy Markdown
Contributor

esasse commented May 5, 2019

Closing due to no response from the author.

@esasse esasse closed this May 5, 2019
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.

5 participants