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

Fix barrier protocol incompatability with older jansson versions and minor cleanup #889

Merged
merged 3 commits into from Nov 1, 2016

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Nov 1, 2016

Please note comment change in 86c650d. I'm pretty sure this is a correct change given the code below it, but would like a double check.

@garlick garlick added the review label Nov 1, 2016

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Nov 1, 2016

Looks perfect, and comment change is correct.

Minor, but commit messages should probably all have the same prefix, like "modules/barrier:"

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 1, 2016

Coverage Status

Coverage decreased (-0.008%) to 75.774% when pulling 5cd28b6 on chu11:barrierhang into 53e1972 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 1, 2016

Current coverage is 72.16% (diff: 100%)

Merging #889 into master will decrease coverage by <.01%

@@             master       #889   diff @@
==========================================
  Files           156        156          
  Lines         26935      26934     -1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          19439      19437     -2   
- Misses         7496       7497     +1   
  Partials          0          0          
Diff Coverage File Path
•••••••••• 100% src/modules/barrier/libbarrier.c
•••••••••• 100% src/modules/barrier/barrier.c

Powered by Codecov. Last update 53e1972...5e44d20

chu11 added some commits Nov 1, 2016

modules/barrier: remove optional key in protocol
Earlier versions of jansson do not allow unpacks to support
strict and optional keys at the same time, causing issues
with the optional "hopcount" key in the barrier module.

To deal with this, make "hopcount" key required and update
use accordingly.

Fixes #888
modules/barrier: rename hopcount to internal
"hopcount" is no longer used as an actual count of hops.  Rename
to "internal" as a flag indicating if a message came from a client
or a downstream barrier.
@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Nov 1, 2016

@garlick Ahh, I see how you guys tend to prefix log messages. Will tweak and push.

@chu11 chu11 force-pushed the chu11:barrierhang branch from 5cd28b6 to 5e44d20 Nov 1, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 1, 2016

Coverage Status

Coverage decreased (-0.005%) to 75.778% when pulling 5e44d20 on chu11:barrierhang into 53e1972 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Nov 1, 2016

Thanks!

@garlick garlick merged commit ef819d7 into flux-framework:master Nov 1, 2016

4 checks passed

codecov/patch 100% of diff hit (target 72.17%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +27.82% compared to 53e1972
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.005%) to 75.778%
Details

@garlick garlick removed the review label Nov 1, 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.