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: Change "namespace" to "name_space" in external header #1327

Merged
merged 1 commit into from Feb 9, 2018

Conversation

Projects
None yet
5 participants
@morrone
Copy link
Contributor

morrone commented Feb 6, 2018

"namespace" is a keyword in C++, so flux will want to avoid using
that in public headers. We change the function parameter name
in kvs.h from "namespace" to "name_space" to reallow C++ linkage.

@morrone morrone requested a review from chu11 Feb 6, 2018

@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Feb 6, 2018

Doh! Change in principal is fine, although my anal-retentive side immediately wondered about changing it to "name_space" everywhere. But that's more than I think is reasonable.

Perhaps just add a comment saying "don't call parameter 'namespace' to avoid conflict w/ C++ keyword", so we know this is a special case thing for just this location.

@morrone

This comment has been minimized.

Copy link
Contributor Author

morrone commented Feb 9, 2018

Your wish is my command. :)

@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage increased (+0.07%) to 78.623% when pulling 658e4f5 on morrone:name_space into 7ce22e2 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 9, 2018

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1327      +/-   ##
==========================================
+ Coverage   78.22%   78.23%   +0.01%     
==========================================
  Files         156      156              
  Lines       28315    28315              
==========================================
+ Hits        22150    22153       +3     
+ Misses       6165     6162       -3
Impacted Files Coverage Δ
src/common/libflux/response.c 83.73% <0%> (-0.82%) ⬇️
src/broker/overlay.c 73.88% <0%> (-0.32%) ⬇️
src/modules/kvs/kvs.c 65.61% <0%> (-0.18%) ⬇️
src/common/libflux/future.c 88.78% <0%> (ø) ⬆️
src/common/libflux/message.c 81.36% <0%> (ø) ⬆️
src/common/libflux/reactor.c 93.73% <0%> (+0.28%) ⬆️
src/common/libutil/base64.c 95.77% <0%> (+0.7%) ⬆️
src/common/libflux/msg_handler.c 87.72% <0%> (+0.72%) ⬆️
src/connectors/local/local.c 88.14% <0%> (+0.74%) ⬆️
src/broker/modservice.c 80.58% <0%> (+0.97%) ⬆️
... and 1 more
kvs: Change "namespace" to "name_space" in external header
"namespace" is a keyword in C++, so flux will want to avoid using
that in public headers.  We change the function parameter name
in kvs.h from "namespace" to "name_space" to reallow C++ linkage.
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Feb 9, 2018

Restarted a couple builders. Thanks @morrone.

@garlick garlick merged commit fafcf6a into flux-framework:master Feb 9, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 78.623%
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.