Skip to content

deprecate blacklist/whitelist in favor of include/exclude#31569

Merged
alalazo merged 1 commit intodevelopfrom
deprecate-blacklist
Jul 14, 2022
Merged

deprecate blacklist/whitelist in favor of include/exclude#31569
alalazo merged 1 commit intodevelopfrom
deprecate-blacklist

Conversation

@tgamblin
Copy link
Copy Markdown
Member

@tgamblin tgamblin commented Jul 13, 2022

For a long time the module configuration has had a few settings that use blacklist/whitelist terminology. We've been asked by some of our users to replace this with more inclusive language. In addition to being non-inclusive, blacklist and whitelist are inconsistent with the rest of Spack, which uses include and exclude for the same concepts.

  • Deprecate blacklist, whitelist, blacklist_implicits and environment_blacklist in favor of exclude, include, exclude_implicits and exclude_env_vars in module configuration, to be removed in Spack v0.20.
  • Print deprecation warnings if any of the deprecated names are in module config.
  • Update tests to test old and new names.
  • Update docs.
  • Update spack config update to fix this automatically, and include a note in the error that you can use this command.

@hnamLANL FYI -- sorry this took so long

@spackbot-app spackbot-app bot added binary-packages commands documentation Improvements or additions to documentation modules tests General test capability(ies) utilities labels Jul 13, 2022
@tgamblin tgamblin requested a review from tldahlgren July 13, 2022 22:41
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some preliminary feedback/questions.

@tgamblin tgamblin force-pushed the deprecate-blacklist branch from 6e4a587 to 4659423 Compare July 13, 2022 23:27
@tgamblin
Copy link
Copy Markdown
Member Author

Added notes on the deprecation version to all the @pytest.mark.parametrize decorators

@tgamblin tgamblin requested a review from tldahlgren July 13, 2022 23:28
@tgamblin tgamblin force-pushed the deprecate-blacklist branch 2 times, most recently from daa3330 to 4f7c66a Compare July 13, 2022 23:40
@tgamblin tgamblin requested a review from becker33 July 13, 2022 23:40
becker33
becker33 previously approved these changes Jul 13, 2022
alalazo
alalazo previously approved these changes Jul 14, 2022
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only two minor comments. The one on the early exit is probably an edge case.

For a long time the module configuration has had a few settings that use
`blacklist`/`whitelist` terminology. We've been asked by some of our users to replace
this with more inclusive language. In addition to being non-inclusive, `blacklist` and
`whitelist` are inconsistent with the rest of Spack, which uses `include` and `exclude`
for the same concepts.

- [x] Deprecate `blacklist`, `whitelist`, `blacklist_implicits` and `environment_blacklist`
      in favor of `exclude`, `include`, `exclude_implicits` and `exclude_env_vars` in module
      configuration, to be removed in Spack v0.20.
- [x] Print deprecation warnings if any of the deprecated names are in module config.
- [x] Update tests to test old and new names.
- [x] Update docs.
- [x] Update `spack config update` to fix this automatically, and include a note in the error
      that you can use this command.
@tgamblin tgamblin dismissed stale reviews from alalazo and becker33 via 24629ab July 14, 2022 18:57
@tgamblin tgamblin force-pushed the deprecate-blacklist branch from 4f7c66a to 24629ab Compare July 14, 2022 18:57
@tgamblin tgamblin requested review from alalazo, becker33 and haampie July 14, 2022 18:57
@alalazo alalazo enabled auto-merge (squash) July 14, 2022 20:19
@alalazo alalazo merged commit 3d0347d into develop Jul 14, 2022
@alalazo alalazo deleted the deprecate-blacklist branch July 14, 2022 20:42
bhatiaharsh pushed a commit to bhatiaharsh/spack that referenced this pull request Aug 8, 2022
…ack#31569)

For a long time the module configuration has had a few settings that use
`blacklist`/`whitelist` terminology. We've been asked by some of our users to replace
this with more inclusive language. In addition to being non-inclusive, `blacklist` and
`whitelist` are inconsistent with the rest of Spack, which uses `include` and `exclude`
for the same concepts.

- [x] Deprecate `blacklist`, `whitelist`, `blacklist_implicits` and `environment_blacklist`
      in favor of `exclude`, `include`, `exclude_implicits` and `exclude_env_vars` in module
      configuration, to be removed in Spack v0.20.
- [x] Print deprecation warnings if any of the deprecated names are in module config.
- [x] Update tests to test old and new names.
- [x] Update docs.
- [x] Update `spack config update` to fix this automatically, and include a note in the error
      that you can use this command.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binary-packages commands documentation Improvements or additions to documentation modules tests General test capability(ies) utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants