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.

Sign up for GitHub

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

Jump to bottom

Fix one more keyword argument warning #251

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Fix one more keyword argument warning #251

merged 1 commit into from
Jan 28, 2020

Conversation

tmatilai
Copy link
Contributor

At least one more warning escaped #248.

olleolleolle
olleolleolle approved these changes Jan 28, 2020
View reviewed changes
Copy link
Collaborator

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

image

This change leaves the test suite with no warnings.

@tmatilai
Copy link
Contributor Author

This change leaves the test suite with no warnings.

Yes, unfortunately that method does not have tests...

@olleolleolle
Copy link
Collaborator

olleolleolle commented Jan 28, 2020 •
edited
Loading

Is it possible to add a test, in this PR?

@tmatilai
Copy link
Contributor Author

Hmm, what would you like it to test? Anyway, I think it has been something like 10 years since I used Minitest last time, so would take me some time to study how to mock things etc.

@leejarvis
Copy link
Owner

leejarvis commented Jan 28, 2020 •
edited
Loading

Thanks @tmatilai -- I don't actually receive any warnings before this change when running the test suite on Ruby 2.7. Should I?

@leejarvis
Copy link
Owner

leejarvis commented Jan 28, 2020 •
edited
Loading

Oh sorry I misunderstood the last comment. There's no coverage so the tests don't show a warning. Don't worry about adding a test, I can do that. Happy to merge this @olleolleolle

EDIT: I've added a test

leejarvis added a commit that referenced this pull request Jan 28, 2020
@leejarvis
Add basic test for Slop.parse
0a44dfe 
See #251
@olleolleolle
Copy link
Collaborator

Thanks, everyone! Glad that we got good outcomes!

@leejarvis leejarvis merged commit 36431f7 into leejarvis:master Jan 28, 2020
@tmatilai tmatilai deleted the keyword_arg branch January 28, 2020 14:32
@ankane
Copy link
Contributor

ankane commented Mar 31, 2020

Just fyi to other users: you can use $VERBOSE = nil to disable this warning until a new version is released.

@leejarvis
Copy link
Owner

I can see that getting annoying so I've released 4.8.1. Thanks for the bump, @ankane

@ankane
Copy link
Contributor

ankane commented Mar 31, 2020

Even better, thanks @leejarvis

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 27, 2020
misc/ruby-slop: update to 4.8.1
66037f6 
Update ruby-slop to 4.8.1.


v4.8.1 (2020-03-31)
-------------------

Bug fixes:
  * Fix keyword argument
    warning. [#251](leejarvis/slop#251)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olleolleolle olleolleolle olleolleolle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tmatilai @olleolleolle @leejarvis @ankane