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

Added multiple argument handling to gem list #604

Merged
merged 3 commits into from Sep 16, 2013

Conversation

zachrab
Copy link
Contributor

@zachrab zachrab commented Jul 21, 2013

@drbrain hopefully this is it! No failing tests and operates as expected!

@cmd.execute
end

assert_equal '', @ui.error
Copy link
Member

Choose a reason for hiding this comment

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

you should also assert that both a and b appear in the output, perhaps assert_match %r%^a %, @ui.output and the same for b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drbrain will do however the assert_match statement is not clear to me...%r%^a %?

Copy link
Member

Choose a reason for hiding this comment

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

you get warnings with /.../ regexps without using (). %r%…% is an alternate way of writing the regexp and avoids the warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drbrain got it. good to know.

@drbrain
Copy link
Member

drbrain commented Jul 30, 2013

This looks great! My only remaining issue is that "name" should be "names" in the top block of QueryCommand#execute because it is an Array of potentially multiple items.

I will merge this after RubyGems 2.1 is released.

drbrain added a commit that referenced this pull request Sep 16, 2013
gem list now lists results for multiple arguments
@drbrain drbrain merged commit db4cc58 into rubygems:master Sep 16, 2013
drbrain added a commit that referenced this pull request Sep 16, 2013
drbrain added a commit that referenced this pull request Sep 16, 2013
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.

None yet

2 participants