-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
config: apply --override-ini eagerly
#13830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| inicfg.update(ini_overrides) | ||
|
|
||
| assert rootdir is not None | ||
| return rootdir, inipath, inicfg or {}, ignored_config_files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated cleanup, the or isn't needed.
nicoddemus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Perhaps mention this in a trivial changelog just in case there is a corner case affected by this?
Currently ini overrides are handled lazily in `getini`, but I think it just complicates things and I don't see a reason to it lazily. Do it eagerly in `determine_setup` instead.
44a7fc8 to
15c2e19
Compare
We don't have "trivial" but I added a "misc" one. |
|
FWIW, this also changes behavior (in a good way!). Before this change, We should probably have a test for this, will try to follow up tomorrow. edit: #13980 |
This was ignored until pytest-dev#13830. With that change, it now gets correctly surfaced to the user as a warning (or error with --strict-config), so we should have a test for it.
This was ignored until #13830. With that change, it now gets correctly surfaced to the user as a warning (or error with --strict-config), so we should have a test for it.
…nvalid option (#13994) * config: Add a test for -o with invalid option (#13980) This was ignored until #13830. With that change, it now gets correctly surfaced to the user as a warning (or error with --strict-config), so we should have a test for it. (cherry picked from commit 9f992b8) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Florian Bruhin <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Currently ini overrides are handled lazily in
getini, but I think it just complicates things and I don't see a reason to do it lazily.Do it eagerly in
determine_setupinstead.