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

flux_reactor_run() should return active watcher count #1085

Merged
merged 2 commits into from Jun 7, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Jun 7, 2017

This PR was split out of PR #1083. It alters the return value of flux_reactor_run(). On success, return the active watcher count so >= 0 indicates success, not just zero. Drop the code that returned -1 with errno = EWOULDBLOCK if the FLUX_REACTOR_NOWAIT or FLUX_REACTOR_ONCE flag was specified as agreed in #963.

garlick added some commits Jun 5, 2017

libflux/reactor: flux_reactor_run return watcher count
Alter the return value of flux_reactor_run() to return the
number of active watchers on success, or -1 on failure.

Update tests that check for 0 as success to allow for >= 0.

Fixes #963
doc/flux_reactor_create(3): clarify NOWAIT, return value
Break some >=80 column lines and remove unnecessary indent.

Reword NOWAIT flag description and RETURN VALUE section.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 7, 2017

Coverage Status

Coverage decreased (-0.03%) to 78.116% when pulling 25457a6 on garlick:reactor_run into d453552 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jun 7, 2017

Codecov Report

Merging #1085 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
- Coverage   77.89%   77.86%   -0.03%     
==========================================
  Files         150      150              
  Lines       25934    25930       -4     
==========================================
- Hits        20200    20190      -10     
- Misses       5734     5740       +6
Impacted Files Coverage Δ
src/common/libflux/reactor.c 92.81% <100%> (-0.09%) ⬇️
src/modules/kvs/kvs.c 79.14% <0%> (-0.65%) ⬇️
src/common/libflux/handle.c 84.81% <0%> (-0.58%) ⬇️
src/common/libflux/message.c 81.33% <0%> (-0.12%) ⬇️
src/common/libflux/request.c 89.04% <0%> (ø) ⬆️
src/common/libutil/dirwalk.c 94.07% <0%> (ø) ⬆️
src/common/libflux/response.c 84.61% <0%> (+0.85%) ⬆️
src/broker/modservice.c 80.19% <0%> (+0.99%) ⬆️

@grondo grondo merged commit 8bd3860 into flux-framework:master Jun 7, 2017

4 checks passed

codecov/patch 100% of diff hit (target 77.89%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +22.1% compared to d453552
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 78.116%
Details
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jun 7, 2017

I didn't notice this before I merged, but does the comment for FLUX_REACTOR_NOWAIT flag in reactor.h need to be changed as well?

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Jun 7, 2017

oops, sure does. I'll just submit that as a separate PR.

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jun 7, 2017

Thanks. Sorry I missed that in review :-(

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Jun 7, 2017

No prob - would have been better if I'd caught it in the first place!

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

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.