Merged
Conversation
Member
adamjstewart
left a comment
adamjstewart
left a comment
There was a problem hiding this comment.
Very minor suggestions, otherwise looks good
Flux uses a fork of ZeroMQ's Collective Code Construction Contract (https://github.com/flux-framework/rfc/blob/master/spec_1.adoc). This model requires a repository fork for every stable release that has patch releases. For example, 0.8.0 and 0.9.0 are both tags within the main repository, but 0.8.1 and 0.9.5 would be releases on the v0.8 and v0.9 forks, respectively.
SteVwonder
force-pushed
the
flux-v0.11.1-0.13.0
branch
2 times, most recently
from
f7efaec to
030ff28
Compare
Member
Author
|
Thanks @adamjstewart. I addressed your three comments. I tacked on a few extra commits with fixes to issues that I ran into while digging into flux-framework/flux-core#2491 and flux-framework/flux-core#2456. I suspect you'll want to peek at those. They are the optional test dependency, |
SteVwonder
force-pushed
the
flux-v0.11.1-0.13.0
branch
from
cba6160 to
2ea1ea4
Compare
adamjstewart
reviewed
Nov 4, 2019
Member
adamjstewart
left a comment
adamjstewart
left a comment
There was a problem hiding this comment.
I think this is the last change and then we're good to merge!
Now that spack#1983 has been merged, master > 0.X.0.
Replace `when=@:0.11.99` with `when=@:0.11` since the intention is to include all patch versions of `0.11`.
In spack#13411, `setup_environment` was split into `setup_build_environment` and `setup_run_environment`, with the `spack_env` and `run_env` arguments being changed to `env`. Somehow the flux package was the only one to not have its `spack_env` references in the function changed to `env`.
with older versions of Flux (i.e, 0.0:0.13), FLUX_CONNECTOR_PATH must be set by spack to prevent failures in certain scenarios (flux-framework/flux-core#2456). the flux binary also sets some other environment variables, which can be listed by running `flux -v start`. I added a few of those just to be sure that the Spack-installed paths are used, rather than system-installed ones.
Install optional dependencies to ensure that only spack-installed software is detected and that all tests are run when `spack install --test` is used. Flux's test suite will test for the existance of valgrind, jq, and any MPI installation. If it detects them (even if they are system-installed and outside the spack environment), it will run optional tests against them. I noticed on my machine that the valgrind tests were running against the system-install valgrind.
SteVwonder
force-pushed
the
flux-v0.11.1-0.13.0
branch
from
2ea1ea4 to
cc68a00
Compare
Member
Author
|
Thanks @adamjstewart for the reviews. Once Travis goes green, I think this PR is ready 😄 |
adamjstewart
approved these changes
Nov 4, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support flux-core versions v0.11.1 to v0.11.3 (and associated flux-sched versions).
Encode the C4 repo model in the
url_for_versionfunction to support future patch release (and minor versions once it passes v1.0).