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

[cleanup] use a better variable name for flux_msg_handler_t internally #1283

Merged
merged 2 commits into from Nov 10, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Nov 9, 2017

@chu11 pointed out that using a variable named w to represent a message handler was confusing.

Change to mh where needed.

Perhaps most importantly, change it in the public header and man page.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 9, 2017

Coverage Status

Coverage increased (+0.04%) to 78.591% when pulling c9824f9 on garlick:mh_variable into b989b36 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 9, 2017

Codecov Report

Merging #1283 into master will increase coverage by 0.03%.
The diff coverage is 92.48%.

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
+ Coverage   77.95%   77.99%   +0.03%     
==========================================
  Files         154      154              
  Lines       29086    29084       -2     
==========================================
+ Hits        22674    22683       +9     
+ Misses       6412     6401      -11
Impacted Files Coverage Δ
src/modules/resource-hwloc/resource.c 67.46% <ø> (ø) ⬆️
src/common/libkvs/kvs_watch.c 87.55% <100%> (-0.89%) ⬇️
src/modules/connector-local/local.c 77.38% <100%> (+1.49%) ⬆️
src/modules/kvs/waitqueue.c 85.43% <100%> (ø) ⬆️
src/broker/content-cache.c 73.43% <100%> (ø) ⬆️
src/broker/modservice.c 80.58% <100%> (-0.98%) ⬇️
src/broker/heartbeat.c 86.95% <100%> (ø) ⬆️
src/broker/overlay.c 73.88% <100%> (ø) ⬆️
src/modules/content-sqlite/content-sqlite.c 56.75% <100%> (ø) ⬆️
src/modules/barrier/barrier.c 78.98% <100%> (ø) ⬆️
... and 29 more
@chu11
Copy link
Contributor

chu11 left a comment

awesome. looks good except for the typo/edit error I found. can hit merge once that's fixed.

Also, does flux-sched need similar cleanup for consistency?

};

static void rusage_request_cb (flux_t *h, flux_msg_handler_t *w,
flux_msg_handler_t *mh; }; static void rusage_request_cb (flux_t *h, flux_msg_handler_t *mh,

This comment has been minimized.

@chu11

chu11 Nov 10, 2017

Contributor

something got messed up here, accidental backspace put these on the same line?

This comment has been minimized.

@garlick

garlick Nov 10, 2017

Author Member

Oops! Sheesh. How'd I miss that.

Just force pushed a fix to that one.

garlick added some commits Nov 9, 2017

cleanup: use "mh" for flux_msg_handler_t var
Use a more sensible variable name for flux_msg_handler_t
than "w", etc..

Fixes #1219
doc/flux_msg_handler_create(3): use "mh" in proto
Use "mh" instead of "w" for flux_msg_handler_t in
function prototypes.

Update dictionary.

@garlick garlick force-pushed the garlick:mh_variable branch from c9824f9 to 22ddc37 Nov 10, 2017

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Nov 10, 2017

does flux-sched need similar cleanup for consistency?

Yep, I'll tack that on to the sched component of #1277.

@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Nov 10, 2017

cool, can hit merge when it turns green

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 10, 2017

Coverage Status

Coverage increased (+0.02%) to 78.577% when pulling 22ddc37 on garlick:mh_variable into b989b36 on flux-framework:master.

@chu11 chu11 merged commit 819d8c5 into flux-framework:master Nov 10, 2017

4 checks passed

codecov/patch 92.48% of diff hit (target 77.95%)
Details
codecov/project 77.99% (+0.03%) compared to b989b36
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 78.577%
Details

@garlick garlick deleted the garlick:mh_variable branch Nov 10, 2017

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

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.