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

@quidryan quidryan commented Aug 9, 2016

Copy link
Copy Markdown

@stleary

stleary commented Aug 9, 2016

Copy link
Copy Markdown
Owner

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

@quidryan

quidryan commented Aug 9, 2016

Copy link
Copy Markdown
Author

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

stleary commented Aug 9, 2016

Copy link
Copy Markdown
Owner

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