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

common/libutil: Fix base64 API documentation #1138

Merged
merged 1 commit into from Aug 4, 2017

Conversation

Projects
None yet
5 participants
@chu11
Copy link
Contributor

chu11 commented Aug 3, 2017

base64_encode_block() and base64_decode_block() require dstlen
to be passed in, but the header files say they are optional. Adjust
documentation appropriately.

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Aug 3, 2017

FWIW, I went back and forth on "fix documentation" or "fix function to do what it says". B/c many other functions require dstlen, I just went with that.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Aug 3, 2017

Oh yep that's true. @dun might want to grab that too, since we raided munge for that code.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 3, 2017

Coverage Status

Coverage increased (+0.1%) to 78.022% when pulling cbe0526 on chu11:base64_documentation into 71b40ff on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 3, 2017

Codecov Report

Merging #1138 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1138      +/-   ##
==========================================
+ Coverage   77.57%   77.61%   +0.03%     
==========================================
  Files         157      157              
  Lines       28679    28678       -1     
==========================================
+ Hits        22247    22257      +10     
+ Misses       6432     6421      -11
Impacted Files Coverage Δ
src/common/libkvs/kvs_watch.c 86.47% <0%> (-0.97%) ⬇️
src/common/libflux/attr.c 87.67% <0%> (-0.92%) ⬇️
src/broker/overlay.c 71.67% <0%> (-0.35%) ⬇️
src/common/libflux/message.c 80.7% <0%> (-0.24%) ⬇️
src/bindings/lua/flux-lua.c 81.67% <0%> (-0.1%) ⬇️
src/common/libkvs/kvs.c 65.33% <0%> (+0.25%) ⬆️
src/modules/kvs/kvs.c 63.45% <0%> (+0.26%) ⬆️
src/common/libcompat/rpc.c 94.57% <0%> (+0.72%) ⬆️
src/common/libflux/response.c 84.55% <0%> (+0.81%) ⬆️
src/broker/modservice.c 80.58% <0%> (+0.97%) ⬆️
... and 2 more
@dun

This comment has been minimized.

Copy link

dun commented Aug 4, 2017

You're mixing verb tenses within a sentence. It should read "and sets [dstlen] to the number of bytes written."

common/libutil: Fix base64 API documentation
base64_encode_block() and base64_decode_block() require dstlen
to be passed in, but the header files say they are optional.  Adjust
documentation appropriately.

@chu11 chu11 force-pushed the chu11:base64_documentation branch from cbe0526 to e5bf5aa Aug 4, 2017

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Aug 4, 2017

fixed and re-pushed

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 4, 2017

Coverage Status

Coverage increased (+0.03%) to 77.96% when pulling e5bf5aa on chu11:base64_documentation into 9e5af83 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Aug 4, 2017

Thanks guys.

@garlick garlick merged commit 38fed2d into flux-framework:master Aug 4, 2017

4 checks passed

codecov/patch Coverage not affected when comparing 9e5af83...e5bf5aa
Details
codecov/project 77.61% (+0.03%) compared to 9e5af83
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 77.96%
Details

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