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

Fix a couple of bundler issues with keyword argument separation #7337

Merged
1 commit merged into from Sep 1, 2019

Conversation

jeremyevans
Copy link
Contributor

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

make test-bundler in Ruby repository failed with recent changes to separate keyword arguments from positional arguments (see https://bugs.ruby-lang.org/issues/14183).

What was your diagnosis of the problem?

Bundler's specs check for lack of warnings, and the new changes cause warnings that cause Bundler's specs to fail.

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

In one case, I fix the code to work with keyword arguments and positional arguments in positional arguments. In the other case, I fix the spec to ignore the keyword argument separation warnings.

I fixed this upstream in Ruby already (ruby/ruby@b5b3afa). So this is a request to make bundler use the same patch. You can certainly fix it differently, and there are more related issues to fix I think (they just don't cause test failures yet). The issues definitely need to be fixed if you want bundler's specs to run correctly in Ruby 2.7.

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

Because it was the least amount of effort.

@hsbt
Copy link
Member

hsbt commented Aug 31, 2019

@jeremy Can you update rubocop warning? https://travis-ci.org/bundler/bundler/jobs/578964090#L618

There are more issues than this, but hopefully this is enough
to get make test-bundler passing in CI.
@jeremyevans
Copy link
Contributor Author

@hsbt Sure, I forced push a commit that should fix it.

@hsbt
Copy link
Member

hsbt commented Sep 1, 2019

@jeremy Thanks, We need to send the same patch to https://github.com/erikhuda/thor as the upstream.

@hsbt
Copy link
Member

hsbt commented Sep 1, 2019

@bundlerbot r+

ghost pushed a commit that referenced this pull request Sep 1, 2019
7337: Fix a couple of bundler issues with keyword argument separation r=hsbt a=jeremyevans

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

`make test-bundler` in Ruby repository failed with recent changes to separate keyword arguments from positional arguments (see https://bugs.ruby-lang.org/issues/14183).

### What was your diagnosis of the problem?

Bundler's specs check for lack of warnings, and the new changes cause warnings that cause Bundler's specs to fail.

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

In one case, I fix the code to work with keyword arguments and positional arguments in positional arguments.  In the other case, I fix the spec to ignore the keyword argument separation warnings.

I fixed this upstream in Ruby already (ruby/ruby@b5b3afa). So this is a request to make bundler use the same patch.  You can certainly fix it differently, and there are more related issues to fix I think (they just don't cause test failures yet).  The issues definitely need to be fixed if you want bundler's specs to run correctly in Ruby 2.7.

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

Because it was the least amount of effort.

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
@ghost
Copy link

ghost commented Sep 1, 2019

Build failed

@deivid-rodriguez
Copy link
Member

@bundlerbot retry

ghost pushed a commit that referenced this pull request Sep 1, 2019
7337: Fix a couple of bundler issues with keyword argument separation r=hsbt a=jeremyevans

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

`make test-bundler` in Ruby repository failed with recent changes to separate keyword arguments from positional arguments (see https://bugs.ruby-lang.org/issues/14183).

### What was your diagnosis of the problem?

Bundler's specs check for lack of warnings, and the new changes cause warnings that cause Bundler's specs to fail.

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

In one case, I fix the code to work with keyword arguments and positional arguments in positional arguments.  In the other case, I fix the spec to ignore the keyword argument separation warnings.

I fixed this upstream in Ruby already (ruby/ruby@b5b3afa). So this is a request to make bundler use the same patch.  You can certainly fix it differently, and there are more related issues to fix I think (they just don't cause test failures yet).  The issues definitely need to be fixed if you want bundler's specs to run correctly in Ruby 2.7.

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

Because it was the least amount of effort.

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
@ghost
Copy link

ghost commented Sep 1, 2019

Build succeeded

@ghost ghost merged commit da7e1f5 into rubygems:master Sep 1, 2019
@deivid-rodriguez deivid-rodriguez added this to the 2.1.0.pre.2 milestone Sep 1, 2019
@jeremyevans jeremyevans deleted the keyword-argument-separation branch September 1, 2019 15:50
deivid-rodriguez pushed a commit that referenced this pull request Sep 15, 2019
7337: Fix a couple of bundler issues with keyword argument separation r=hsbt a=jeremyevans

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

`make test-bundler` in Ruby repository failed with recent changes to separate keyword arguments from positional arguments (see https://bugs.ruby-lang.org/issues/14183).

### What was your diagnosis of the problem?

Bundler's specs check for lack of warnings, and the new changes cause warnings that cause Bundler's specs to fail.

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

In one case, I fix the code to work with keyword arguments and positional arguments in positional arguments.  In the other case, I fix the spec to ignore the keyword argument separation warnings.

I fixed this upstream in Ruby already (ruby/ruby@b5b3afa). So this is a request to make bundler use the same patch.  You can certainly fix it differently, and there are more related issues to fix I think (they just don't cause test failures yet).  The issues definitely need to be fixed if you want bundler's specs to run correctly in Ruby 2.7.

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

Because it was the least amount of effort.

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
(cherry picked from commit 8b61b4b)
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.

None yet

3 participants