Skip to content
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

Correct the spelling of ArgSource.INVOCATION_DIR #11333

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

jparise
Copy link
Contributor

@jparise jparise commented Aug 22, 2023

This was spelled incorrectly when introduced in #9897.

INCOVATION_DIR = enum.auto()
INVOCATION_DIR = enum.auto()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are backwards-compatibility concerns, we could also add an alias:

INCOVATION_DIR = INVOCATION_DIR

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should add an alias, otherwise it'd be a breaking change.

Config.ArgsSource.INCOVATION_DIR remains as a backwards compatibility
alias.
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks (also for adding the tests)!

I'll add a note to the changelog that an alias remains.

changelog/11333.trivial.rst Show resolved Hide resolved
@bluetech bluetech enabled auto-merge (squash) August 23, 2023 08:58
@bluetech bluetech merged commit 7500fe4 into pytest-dev:main Aug 23, 2023
23 checks passed
@jparise jparise deleted the invocation branch August 23, 2023 16:51
@The-Compiler
Copy link
Member

Wouldn't it make sense to use a __getattr__ or somesuch for the alias, so that it can show a deprecation warning and we can remove it in the future?

@bluetech
Copy link
Member

Deprecations == pain, IMO it's not worth it for something like an alias.

@jparise
Copy link
Contributor Author

jparise commented Sep 4, 2023

Wouldn't it make sense to use a __getattr__ or somesuch for the alias, so that it can show a deprecation warning and we can remove it in the future?

I don't have a strong preference either way, but I'd be happy to implement this approach if it's more aligned with the project's norms.

As an aside, the old (misspelled) symbol doesn't appear to have been used by any open source (on GitHub) code: https://github.com/search?q=INCOVATION_DIR+lang%3Apython&type=code

... but that is of course an incomplete view.

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