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

Minor flux-ping cleanup #904

Merged
merged 5 commits into from Nov 15, 2016

Conversation

Projects
None yet
5 participants
@chu11
Copy link
Contributor

chu11 commented Nov 15, 2016

Splitting off these "minor cleanups" of flux-ping from #898.

chu11 added some commits Nov 5, 2016

cmd/flux-ping: Do not wait to send first ping
For consistency to the ping(8) command, do not wait to send out
the first ping, regardless of what the period may be.
cmd/flux-ping: Change period to delay
For consistency to option name, use variable 'delay' instead
of 'period' where appropriate.
cmd/flux-ping: Change --delay to --interval
For consistency to the ping(8) command, rename the --delay option
to the --interval option.

@garlick garlick added the review label Nov 15, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 15, 2016

Coverage Status

Changes Unknown when pulling 2b5fb70 on chu11:fluxpingminorcleanup into * on flux-framework:master*.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 15, 2016

Current coverage is 72.36% (diff: 100%)

Diff Coverage File Path
•••••••••• 100% src/cmd/flux-ping.c

No coverage report found for master at 4b9e43b.

Powered by Codecov. Last update 4b9e43b...2b5fb70

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 15, 2016

Great! One other thing we could do is update flux-ping.c to use optparse instead of getopt_long directly, but that doesn't seem as critical as these cleanups here. Is this ready to go?

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Nov 15, 2016

@grondo I didn't have optparse on my list, but I can add it. Perhaps we'll consider that one non-minor and it'll be on the bigger ticket #898. Or if it becomes larger I'll split off another PR again.

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 15, 2016

Ok, merging then

@grondo grondo merged commit 55dcaaf into flux-framework:master Nov 15, 2016

4 checks passed

codecov/patch 100% of diff hit (target 72.36%)
Details
codecov/project 72.36% (+0.02%) compared to da42f80
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 75.975%
Details

@grondo grondo removed the review label Nov 15, 2016

@grondo grondo referenced this pull request Nov 28, 2016

Closed

Create 0.6.0 release notes #916

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.