Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gem::Requirement.create treat arguments as variable-length #1830

Merged

Conversation

negito6
Copy link
Contributor

@negito6 negito6 commented Jan 31, 2017

Description:

This PR enables Gem::Requirement.create to treat arguments as variable-length.

Now, we can write only

Gem::Requirement.create([">= 1.2", "<= 1.3"])

New style will be supported

Gem::Requirement.create(">= 1.2", "<= 1.3")

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

@negito6
Copy link
Contributor Author

negito6 commented Jan 31, 2017

I will fix the errors.

@segiddins
Copy link
Member

Is this worth the performance penalty?

@drbrain
Copy link
Member

drbrain commented Jan 31, 2017

Yes.

We can write not only `Gem::Requirement.create([">= 1.2", "<= 1.3"])`
but also `Gem::Requirement.create(">= 1.2", "<= 1.3")`
@negito6 negito6 force-pushed the requirement-create-variable-length-arg branch from 771c96b to 6526dd0 Compare February 1, 2017 01:00
@negito6
Copy link
Contributor Author

negito6 commented Feb 1, 2017

Thanks your reviews.

I feel the current style create([ ... ]) is not like Ruby.
and also because add_dependency can be

add_dependency 'example', '>= 1.0.0.a', '< 2.0.0'

But it is only my subjectivity. Positive comments here will show the worth.

@@ -51,7 +51,11 @@ class BadRequirementError < ArgumentError; end
# If the input is "weird", the default version requirement is
# returned.

def self.create input
def self.create *inputs
return create inputs if inputs.length > 1
Copy link
Member

Choose a reason for hiding this comment

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

Why not new inputs? This should match L63 right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly it is useless method call. I fixed it.

@drbrain
Copy link
Member

drbrain commented Feb 1, 2017

@homu r+

@homu
Copy link
Contributor

homu commented Feb 1, 2017

📌 Commit 4455566 has been approved by drbrain

@homu
Copy link
Contributor

homu commented Feb 1, 2017

⌛ Testing commit 4455566 with merge 5e63fd3...

homu added a commit that referenced this pull request Feb 1, 2017
… r=drbrain

Gem::Requirement.create treat arguments as variable-length

# Description:

This PR enables `Gem::Requirement.create` to treat arguments as variable-length.

Now, we can write only
```ruby
Gem::Requirement.create([">= 1.2", "<= 1.3"])
```

New style will be supported
```ruby
Gem::Requirement.create(">= 1.2", "<= 1.3")
```
______________

# Tasks:

- [x] Describe the problem / feature
- [x] Write tests
- [x] Write code to solve the problem
- [x] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
@homu
Copy link
Contributor

homu commented Feb 1, 2017

☀️ Test successful - status

@homu homu merged commit 4455566 into rubygems:master Feb 1, 2017
@negito6
Copy link
Contributor Author

negito6 commented Feb 1, 2017

I appreciate your review and merge ! @segiddins @drbrain

@negito6 negito6 deleted the requirement-create-variable-length-arg branch February 1, 2017 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants