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

kvs: additional cleanup and refactoring #1273

Merged
merged 5 commits into from Nov 5, 2017

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Nov 3, 2017

part three of issue #1264. This is the easy set of cleanups/refactoring. Don't think anything here will be considered too fascinating.

Only thing to note is the old kvs_util_json_has() function took a href_t, but treeobj_hash() takes a char * and int. It's b/c href_t is only defined for the KVS server right now. Considered removing href_t and generalizing it with char ref[NUM], but I elected not too.

chu11 added some commits Nov 3, 2017

modules/kvs: Refactor cache_entry_get_raw()
Function now takes const void ** isntead of just void **.

Adjust callers appropriately.
common/libkvs/treeobj: Add treeobj_hash()
Add new function to hash tree objects.  Add unit tests appropriately.
modules/kvs: Remove kvs_util_json_hash()
Replace with call to treeobj_hash().
modules/kvs: Cleanup code in commit_unroll()
With RFC11 changes, we always know the data in a treeobj val object
is a base64 encoded string.  Therefore, we do not need to call
kvs_util_json_encoded_size to get the encoded size.  We just
need to call strlen() on the string value.
modules/kvs/kvs_util: Remove unused functions
With recent changes, kvs_util_json_dumps() and
kvs_util_json_encoded_size() are no longer used, so remove
them and associated unit tests.

Also remove now unnecessary header includes.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 4, 2017

Coverage Status

Coverage increased (+0.007%) to 78.507% when pulling bebc347 on chu11:issue1264-part3 into 85353aa on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 4, 2017

Codecov Report

Merging #1273 into master will increase coverage by 0.01%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master    #1273      +/-   ##
==========================================
+ Coverage   77.87%   77.88%   +0.01%     
==========================================
  Files         154      154              
  Lines       29138    29124      -14     
==========================================
- Hits        22691    22684       -7     
+ Misses       6447     6440       -7
Impacted Files Coverage Δ
src/modules/kvs/lookup.c 80.47% <ø> (ø) ⬆️
src/modules/kvs/kvs_util.c 100% <ø> (+12.24%) ⬆️
src/modules/kvs/commit.c 74.78% <100%> (+0.21%) ⬆️
src/common/libkvs/treeobj.c 86.2% <100%> (+0.66%) ⬆️
src/modules/kvs/cache.c 88.72% <100%> (ø) ⬆️
src/modules/kvs/kvs.c 62.62% <66.66%> (+0.25%) ⬆️
src/common/libflux/content.c 86.66% <0%> (-3.34%) ⬇️
src/common/libflux/rpc.c 92.56% <0%> (-0.83%) ⬇️
src/broker/overlay.c 73.88% <0%> (-0.64%) ⬇️
src/common/libflux/msg_handler.c 86.09% <0%> (-0.38%) ⬇️
... and 7 more
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Nov 5, 2017

Looks great, thanks.

@garlick garlick merged commit d7300ec into flux-framework:master Nov 5, 2017

4 checks passed

codecov/patch 95.23% of diff hit (target 77.87%)
Details
codecov/project 77.88% (+0.01%) compared to 85353aa
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 78.507%
Details

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