Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Logging warnings when unsupported config options are set #5

Closed
enpaul opened this issue Sep 29, 2020 · 2 comments · Fixed by #32
Closed

Logging warnings when unsupported config options are set #5

enpaul opened this issue Sep 29, 2020 · 2 comments · Fixed by #32
Labels
feature New feature or request

Comments

@enpaul
Copy link
Owner

enpaul commented Sep 29, 2020

There are a number of options Tox provides that are fairly specific to configuring how Tox interacts with Pip. These options have no usage when installing using Poetry because Poetry replaces the functionality these options are used to configure (there are exceptions however, see #4).

When the require_locked_deps = true setting is set for a Tox environment the plugin should log a warning for each of these unsupported config options that are also in use for the environment. These should not be breaking, but a notice should be given to the user that a setting they specified is being ignored. I think this is preferable to having unsupported (and inherently unusable) settings cause the environment to fail.

A non-exhaustive list of settings this plugin should warn on is below:

Note: If the required_locked_deps = false option is set then the warnings should not be raised. This is because these settings can and will still impact installations that use the default Tox installation backend, which may be in use if the setting is not true.

@enpaul enpaul added roadmap-beta feature New feature or request and removed roadmap-beta labels Sep 29, 2020
@enpaul
Copy link
Owner Author

enpaul commented Oct 22, 2020

The requires option should be added to this list. See #13 for explanation of why the requires isn't the best idea.

That issue deals covers adding documentation for users about why not to use requires, but there should also be a warning-level log message raised when the requires config option is present. Unlike the other options mentioned above, the requires config option is a config-global parameter so a warning should be raised if it used at all, regardless of whether the specific env uses require_locked_deps.

Note that this warning should be raised after the "project uses poetry" check, to keep with non-poetry project support as noted in #1

@enpaul enpaul mentioned this issue Dec 5, 2020
@enpaul enpaul closed this as completed in #32 Dec 5, 2020
@enpaul
Copy link
Owner Author

enpaul commented Dec 5, 2020

This feature was determined to be out of scope. Tox does not provide a way to distinguish between a setting that is not set in the config and as simply taken on the default value and a setting that is set in the config. In addition, because there is no programatically accessible reference of Tox's configuration settings, these values can't be computed by comparing existing config values to their default state.

Implementing this feature would require a not-insubstantial amount of work to either re-implement or independently call the parsing logic for Tox's config. That sounds like a really bad idea so I don't think I'm going to do it. In the meantime I've compromised with more thorough documentation on the unsupported options and the behaviour changes that the settings impact.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant