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

garlick
Copy link
Member

@garlick 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!

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
Copy link
Contributor

morrone commented Oct 18, 2017

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

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

@dongahn
Copy link
Member

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
Copy link
Contributor

grondo commented Oct 18, 2017

flux-framework/flux-core#1233 is now merged.

@coveralls
Copy link

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
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
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
@lipari
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 mentioned this pull request May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants