Skip to content

Improving opt test to include missing values#54

Closed
quidryan wants to merge 1 commit into
stleary:masterfrom
quidryan:optBoolean-missing-value
Closed

Improving opt test to include missing values#54
quidryan wants to merge 1 commit into
stleary:masterfrom
quidryan:optBoolean-missing-value

Conversation

@quidryan
Copy link
Copy Markdown

@quidryan quidryan commented Aug 9, 2016

@stleary
Copy link
Copy Markdown
Owner

stleary commented Aug 9, 2016

Will consider after the revert is completed. Thanks again for catching it.

@quidryan
Copy link
Copy Markdown
Author

quidryan commented Aug 9, 2016

I wasn't following the previous work that had to be reverted, but wouldn't this test be relevant irrelevant to the state of the JSON-Java repo? This add coverage which did not exist before.

@stleary
Copy link
Copy Markdown
Owner

stleary commented Aug 9, 2016

Well, the tests are not even included in the Maven repo. We keep them release-synced, so there is always a test release to match the JSON release, but there is no requirement for them to be timestamp-synced, if we are rushed. Since @johnjaylward has also made a pull request in #55, we will proceed there. Please take a look at that branch and post if you see any tests that should be added.

@stleary stleary closed this Aug 9, 2016
@johnjaylward
Copy link
Copy Markdown
Contributor

@quidryan , I basically did the same as you, but extended the tests to cover more opt methods in #55

I believe my PR has more coverage than your even though we took the same approach. Let me know if you see anything else that needs to be added.

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