Removed default value for the function argument 'dirty'#5109
Merged
tgamblin merged 1 commit intospack:developfrom Aug 26, 2017
Merged
Removed default value for the function argument 'dirty'#5109tgamblin merged 1 commit intospack:developfrom
tgamblin merged 1 commit intospack:developfrom
Conversation
This change is done to avoid inconsistencies during refactoring. The rationale is that functions at different levels in the call stack all define a default for the 'dirty' argument. This PR removes the default value for all the functions except the top-level one (`PackageBase.do_install`). In this way not defining 'dirty' will result in an error, instead of the default value being used. This will reduce the risk of having an inconsistent behavior after a refactoring.
tgamblin
approved these changes
Aug 26, 2017
Contributor
|
I believe that this PR made it so that the spack setup command no longer works the way it used to. The help for the command does not indicate any second argument. Is it now required that we pass the --clean or --dirty flag? |
alalazo
added a commit
to epfl-scitas/spack
that referenced
this pull request
Sep 4, 2017
This command broke after spack#5109. It was using the default value for the "dirty" argument in `setup_package`. Now it adopts the same logic as in `spack install`. Trivial unit test included.
alalazo
added a commit
that referenced
this pull request
Sep 5, 2017
This command broke after #5109. It was using the default value for the "dirty" argument in `setup_package`. Now it adopts the same logic as in `spack install`. Changed help for '--clean' and '--dirty'. Improved coverage of spack env.
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
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.
This change is done to avoid inconsistencies during refactoring. The rationale is that functions at different levels in the call stack all define a default for the 'dirty' argument. This PR removes the default value for all the functions except the top-level one (
PackageBase.do_install).In this way not defining 'dirty' will result in an error, instead of the default value being used. This will reduce the risk of having an inconsistent behavior after a refactoring.