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

libschedutil: remove vendored copy and use flux-core's exported lib #516

Merged
merged 1 commit into from Sep 24, 2019

Conversation

@SteVwonder
Copy link
Member

SteVwonder commented Sep 18, 2019

  • remove the copy in src/common/libschedutil and leverage the libschedutil exported by flux-core
  • update qmanager's usage of the schedutil library to match the new flux-core libschedutil API

This won't pass Travis tests until flux-framework/flux-core#2380 is merged.

@SteVwonder SteVwonder requested review from grondo and dongahn Sep 18, 2019
@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 19, 2019

Now flux-framework/flux-core#2380 is merged, kick the Travis CI for this PR again?

@SteVwonder

This comment has been minimized.

Copy link
Member Author

SteVwonder commented Sep 19, 2019

Yep! Just restarted the CI. 🤞

@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 19, 2019

Travis is compiling about being unable to find a makefile

make clean...0.00s
make: *** No targets specified and no makefile found.  Stop.
docker-run-checks.sh: docker run failed
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 19, 2019

Codecov Report

Merging #516 into master will increase coverage by 0.49%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
+ Coverage   76.05%   76.55%   +0.49%     
==========================================
  Files          63       58       -5     
  Lines        6503     6291     -212     
==========================================
- Hits         4946     4816     -130     
+ Misses       1557     1475      -82
Impacted Files Coverage Δ
qmanager/modules/qmanager.cpp 70.48% <75%> (-0.08%) ⬇️

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 a317aad...ff1f54a. Read the comment docs.

@SteVwonder

This comment has been minimized.

Copy link
Member Author

SteVwonder commented Sep 19, 2019

@dongahn : Travis is happy now. I think I restarted the builder too soon after the flux-core PR was merged, so it pull an older docker image.

That being said, I am happy to wait and merge this one after #513 goes in.

@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 20, 2019

@SteVwonder: This looks great. I really appreciate the fact that I won't have to manually track schedutil any longer -- copying the source from flux-core to flux-sched anytime there is a change.

I found no fault in this PR. Once you rebase this PR to the current master that has @tpatki's get/set-property support, this can go in.

remove the copy in `src/common/libschedutil` and leverage the
libschedutil exported by flux-core

update qmanager's usage of the schedutil library to match the new flux-core
libschedutil API
@SteVwonder SteVwonder force-pushed the SteVwonder:schedutil-from-core branch from 7f2dd42 to ff1f54a Sep 24, 2019
@SteVwonder

This comment has been minimized.

Copy link
Member Author

SteVwonder commented Sep 24, 2019

Thanks @dongahn. Just rebased and force-pushed.

@dongahn dongahn merged commit 77d7438 into flux-framework:master Sep 24, 2019
2 of 3 checks passed
2 of 3 checks passed
codecov/patch 75% of diff hit (target 76.05%)
Details
codecov/project 76.55% (+0.49%) compared to a317aad
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 24, 2019

Just merged. Thanks.

@SteVwonder SteVwonder deleted the SteVwonder:schedutil-from-core branch Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.