Skip to content

Removed default value for the function argument 'dirty'#5109

Merged
tgamblin merged 1 commit intospack:developfrom
epfl-scitas:fixes/remove_defaults_for_dirty
Aug 26, 2017
Merged

Removed default value for the function argument 'dirty'#5109
tgamblin merged 1 commit intospack:developfrom
epfl-scitas:fixes/remove_defaults_for_dirty

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Aug 15, 2017

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.

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.
@alalazo alalazo added the ready label Aug 15, 2017
@alalazo alalazo requested a review from becker33 August 15, 2017 19:22
@tgamblin tgamblin merged commit 005b22a into spack:develop Aug 26, 2017
@alalazo alalazo deleted the fixes/remove_defaults_for_dirty branch August 26, 2017 06:44
@bvanessen
Copy link
Copy Markdown
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.
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.

3 participants