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

track KVS API changes in flux-core #271

Merged
merged 1 commit into from Oct 19, 2017

Conversation

Projects
None yet
7 participants
@garlick
Copy link
Member

garlick commented Oct 18, 2017

This PR tracks flux-core API changes proposed in flux-framework/flux-core#1233. It should not be merged until after that one (and sched travis-ci can be shown to run successfully)

The first commit simply renames functions and types to get sched to comple.

The rest of the commits migrate sched off of functions that now cause "deprecated" warnings from the compiler. In the process, I did some minor refactoring and error handling cleanup in the simulator code surrounding KVS access to job data. I hope it's not a pain to review @SteVwonder!

Track libkvs API changes in flux-core
All of the KVS functions and typedefs in flux-core are now
namespaced with a flux_ prefix.  Several functions were
also renamed, such as
   kvs_commit() -> flux_kvs_commit_anon()

Finally, the kvsdir_put_<type> functions were replaced
with flux_kvsdir_pack().
@morrone

This comment has been minimized.

Copy link
Contributor

morrone commented Oct 18, 2017

It sounds like the simulator cleanup should perhaps be a separate pull request?

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Oct 18, 2017

Good idea, that way @SteVwonder can take his time with the cleanup review.

There will be deprecated function warnings until then, but not a big deal.

@garlick garlick force-pushed the garlick:kvs_api_names branch from c181e29 to c02181c Oct 18, 2017

@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Oct 18, 2017

Like the plan. @SteVwonder will be busy throughout this week for IPDPS.

@garlick garlick changed the title track KVS API changes in flux-core, minor simulator cleanup track KVS API changes in flux-core Oct 18, 2017

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 18, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 18, 2017

Coverage Status

Coverage remained the same at 74.67% when pulling c02181c on garlick:kvs_api_names into be23ee7 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 18, 2017

Codecov Report

Merging #271 into master will not change coverage.
The diff coverage is 97.43%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #271   +/-   ##
=======================================
  Coverage   72.17%   72.17%           
=======================================
  Files          29       29           
  Lines        5862     5862           
=======================================
  Hits         4231     4231           
  Misses       1631     1631
Impacted Files Coverage Δ
simulator/sim_execsrv.c 86.45% <100%> (ø) ⬆️
simulator/submitsrv.c 81.08% <100%> (ø) ⬆️
sched/sched.c 75.87% <100%> (ø) ⬆️
simulator/simulator.c 83.69% <96.77%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be23ee7...c02181c. Read the comment docs.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Oct 18, 2017

OK, the flux-core PR was merged and I restarted travis-ci here and tests passed. I think this is ready to merge. Once that's done, I'll rebase #272

@lipari lipari merged commit a5d83bb into flux-framework:master Oct 19, 2017

4 checks passed

codecov/patch 97.43% of diff hit (target 72.17%)
Details
codecov/project 72.17% (+0%) compared to be23ee7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 74.67%
Details
@lipari

This comment has been minimized.

Copy link
Contributor

lipari commented Oct 19, 2017

Thanks, @garlick! You can proceed with the #272 rebase. I'm eager to get flux-sched building again so I can rebase and test my priority branch.

@grondo grondo referenced this pull request May 11, 2018

Closed

Need 0.5.0 Release #340

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.