Skip to content

Conversation

@pelbyl
Copy link
Contributor

@pelbyl pelbyl commented Sep 21, 2024

Replace the decrement (argc2--) with an increment (argc2++) for the correct number of arguments when opt is provided.

@jeremyevans
Copy link
Contributor

Is it possible for you to include a test?

@jeremyevans
Copy link
Contributor

Since you asked via email what I meant by "include a test"?, it would be updating the test code to show that your change fixes the issue. I believe it does, as it appears use of limit keyword is currently broken for the method. You'd probably want to add a test case near

assert_equal('2001-02-03T04:05:06.123+00:00', d2.iso8601(3))

@pelbyl
Copy link
Contributor Author

pelbyl commented Sep 23, 2024 via email

Replace the decrement (argc2--) with an increment (argc2++) for
the correct number of arguments when opt is provided.
@pelbyl
Copy link
Contributor Author

pelbyl commented Sep 23, 2024

@jeremyevans please take a look at the new tests, especially

b6974b0#diff-81966e1c9ca334eec0c25c83971b5ca20f9d52e5535b076db6f288b3a18ef996R415

Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the patch!

@pelbyl pelbyl closed this Oct 28, 2024
@pelbyl pelbyl reopened this Oct 28, 2024
@nobu nobu merged commit 60a7657 into ruby:master Nov 5, 2024
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.

3 participants