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

liboptparse: cleanup, minor fixes, minor additions #911

Merged
merged 7 commits into from Nov 20, 2016

Conversation

Projects
None yet
5 participants
@chu11
Copy link
Contributor

chu11 commented Nov 19, 2016

Another spinoff from #898. WIth the minor exception of the test count in test/optparse.c, I don't believe anything here will conflict with PR #910 .

Most notable fix is optparse_get_int can return negative values, so it's up the caller to check for correctness of the inputted value.

Most notable addition is optparse_get_double convenience function.

chu11 added some commits Nov 18, 2016

liboptparse: Add corner case check
Check errno for error in optparse_get_int()
liboptparse: Allow parse of negative numbers
optparse_get_int() is allowed to parse negative numbers now,
checking for negative required by caller.
test/optparse: Add new optparse_get_int tests
Add tests for negative numbers for optparse_get_int
cmd/flux-start: Adjust optparse_get_int usage
Check return from optparse_get_int, adjusting for fact optparse_get_int
can now return negative numbers.

@garlick garlick added the review label Nov 19, 2016

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 19, 2016

Current coverage is 72.54% (diff: 81.25%)

Merging #911 into master will decrease coverage by 0.01%

@@             master       #911   diff @@
==========================================
  Files           159        159          
  Lines         27159      27173    +14   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          19705      19712     +7   
- Misses         7454       7461     +7   
  Partials          0          0          
Diff Coverage File Path
•••••• 66% src/cmd/flux-start.c
•••••••• 84% src/common/liboptparse/optparse.c

Powered by Codecov. Last update a9ff086...ae9919b

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 19, 2016

Coverage Status

Coverage decreased (-0.02%) to 76.134% when pulling 25a12b9 on chu11:liboptparsecleanup into a9ff086 on flux-framework:master.

@grondo
Copy link
Contributor

grondo left a comment

Nice, just one typo to fix up and then I think this can go straight in. I'll rebase #910 after since it is still in progress.

* Return the option argument as an double if 'name' was used,
* 'default_value' if not. If the option is unknown, or the argument
* could not be converted to an double, call the fatal error function.
*/

This comment has been minimized.

@grondo

grondo Nov 19, 2016

Contributor

Couple typos: "an double" appears twice above

This comment has been minimized.

@chu11

chu11 Nov 19, 2016

Author Contributor

ahh thanks for the catch, i'll fix it up and re-push

chu11 added some commits Nov 17, 2016

test/optparse: Add new tests
Add tests for optparse_get_double
Add test for optparse_get_int to fail on decimal number

@chu11 chu11 force-pushed the chu11:liboptparsecleanup branch from 25a12b9 to ae9919b Nov 19, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 19, 2016

Coverage Status

Coverage decreased (-0.02%) to 76.134% when pulling ae9919b on chu11:liboptparsecleanup into a9ff086 on flux-framework:master.

@grondo grondo merged commit 3defa97 into flux-framework:master Nov 20, 2016

4 checks passed

codecov/patch 81.25% of diff hit (target 72.55%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +8.69% compared to a9ff086
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 76.134%
Details

@grondo grondo removed the review label Nov 20, 2016

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 20, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.