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

Broker cleanup 1 #1230

Merged
merged 3 commits into from Oct 12, 2017

Conversation

Projects
None yet
5 participants
@morrone
Copy link
Contributor

morrone commented Oct 11, 2017

Minor code cleanup in the flux broker.

@garlick
Copy link
Member

garlick left a comment

looks fine to me

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 11, 2017

Coverage Status

Coverage decreased (-0.02%) to 78.622% when pulling f752a76 on morrone:broker_cleanup_1 into 9d3b4b1 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 11, 2017

Codecov Report

Merging #1230 into master will decrease coverage by 0.02%.
The diff coverage is 74.41%.

@@            Coverage Diff            @@
##           master   #1230      +/-   ##
=========================================
- Coverage   78.12%   78.1%   -0.03%     
=========================================
  Files         154     154              
  Lines       28660   28665       +5     
=========================================
- Hits        22391   22388       -3     
- Misses       6269    6277       +8
Impacted Files Coverage Δ
src/broker/broker.c 73% <74.41%> (-0.25%) ⬇️
src/cmd/flux-start.c 74.19% <0%> (-2.51%) ⬇️
src/broker/modservice.c 79.61% <0%> (-0.98%) ⬇️
src/common/libflux/rpc.c 91.73% <0%> (-0.83%) ⬇️
src/common/libflux/message.c 81.6% <0%> (-0.12%) ⬇️
src/modules/kvs/kvs.c 64% <0%> (ø) ⬆️
src/common/libkvs/kvs.c 65.37% <0%> (ø) ⬆️
src/broker/module.c 84.07% <0%> (+0.27%) ⬆️
src/cmd/flux-module.c 84.45% <0%> (+0.3%) ⬆️
... and 2 more
@morrone

This comment has been minimized.

Copy link
Contributor Author

morrone commented Oct 11, 2017

The reported code coverage changes probably aren't real?

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 11, 2017

Yeah, the overall change is minimal and probably just noise. The patch coverage is low probably due to error paths.

This just needs to be rebased on current master

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 12, 2017

Coverage Status

Coverage decreased (-0.06%) to 78.651% when pulling ae965d3 on morrone:broker_cleanup_1 into 5fe021a on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 12, 2017

@morrone I think you'll want to try again with a rebase not a merge to keep the merge commit out of the history.

@morrone

This comment has been minimized.

Copy link
Contributor Author

morrone commented Oct 12, 2017

@garlick, Ah, I was trying github's magic "update branch" button. Apparently that isn't that useful. :)

morrone added some commits Oct 6, 2017

broker: Minor refactoring to improve code isolation
Reduce the number of times that the giant semi-global "ctx" is
passed to functions that only need some small subset of that
data.
broker: Remove dead code
This attribute check for "broker.rundir" would appear to be redundant.
create_rundir() will return an error if the broker.rundir attribute
is not set, and main() will exit long before this check occurs.  It
does not appear that anything else would change broker.rundir later.
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 12, 2017

@garlick, Ah, I was trying github's magic "update branch" button. Apparently that isn't that useful. :)

Yeah, I really wish they had a "rebase branch" button, that would be awesome...

Looks good, I'll merge.

@grondo grondo merged commit c6412bf into flux-framework:master Oct 12, 2017

1 of 3 checks passed

codecov/patch 74.41% of diff hit (target 78.12%)
Details
codecov/project 78.1% (-0.03%) compared to 5fe021a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@morrone morrone deleted the morrone:broker_cleanup_1 branch Oct 12, 2017

@morrone

This comment has been minimized.

Copy link
Contributor Author

morrone commented Oct 13, 2017

Part of the cleanup for issue #1236.

@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.