Add alias "allowlist_externals" to "whitelist_externals" #1601
Conversation
dajose
requested review from
asottile,
gaborbernat and
obestwalter
as code owners
|
I think the macOS checks failure is not related with the PR, can someone give me a hand there? |
src/tox/config/__init__.py
Outdated
| parser.add_testenv_attribute( | ||
| name="whitelist_externals", | ||
| name="whitelist_externals", type="line-list", help="DEPRECATED: use allowlist_external", |
src/tox/config/__init__.py
Outdated
| @@ -747,8 +747,13 @@ def passenv(testenv_config, value): | |||
| "eventual passenv setting.", | |||
| ) | |||
|
|
|||
| # FIXME remove whitelist in favor of allowlist_external | |||
There was a problem hiding this comment.
We don't use fixmes in this project but instead prefer the issue tracker, please remove this. We could not remove it either way before the next major version as that would be breaking change.
docs/changelog/1491.deprecation.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Whitelist (as used on whitelist_externals) has a racial weight, allowlist_externals should be preferred. - by :user:`dajose` | |||
There was a problem hiding this comment.
This needs to expand on the fact that whitelist_externals is still supported, however now the allowlist_externals alias has been added.
src/tox/config/__init__.py
Outdated
| @@ -747,8 +747,13 @@ def passenv(testenv_config, value): | |||
| "eventual passenv setting.", | |||
| ) | |||
|
|
|||
| # FIXME remove whitelist in favor of allowlist_external | |||
There was a problem hiding this comment.
We don't use fixmes in this project but instead prefer the issue tracker, please remove this. We could not remove it either way before the next major version as that would be breaking change.
tests/unit/test_venv.py
Outdated
| @@ -433,6 +433,10 @@ def test_install_command_not_installed(newmocksession): | |||
|
|
|||
|
|
|||
| def test_install_command_whitelisted(newmocksession): | |||
| """ | |||
| Deprecated: Remove in favor of test_install_command_allowlisted when | |||
There was a problem hiding this comment.
Remove this docstring, the migration complete will not happen before the rewrite.
src/tox/venv.py
Outdated
| @@ -223,7 +223,7 @@ def is_allowed_external(self, p): | |||
| if tox.INFO.IS_WIN: | |||
| tryadd += [os.path.normcase(x) for x in os.environ["PATHEXT"].split(os.pathsep)] | |||
| p = py.path.local(os.path.normcase(str(p))) | |||
| for x in self.envconfig.whitelist_externals: | |||
| for x in self.envconfig.allowlist_externals + self.envconfig.whitelist_externals: | |||
There was a problem hiding this comment.
I'm not happy to allow specifying both of them. We should instead allow only one, and raise if both set. And then pick only the one that is set.
|
@gaborbernat can you give me a hand on why it is failing? I think it is not related with the changes: https://dev.azure.com/toxdev/tox/_build/results?buildId=2498&view=logs&j=6f785b48-ed19-5349-e6cb-bb6d29a19c8b&t=4621fbdb-f74d-5491-ece0-e6cd5b96369c&l=704 |
|
re-triggered CI, seems to be doing better now |
closes #1491