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

setup.py template: Correct Python version classifiers #479

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

gotmax23
Copy link
Contributor

This conflicts with #477. I'll rebase that patch to apply the breaking change on top of this PR after this PR is approved.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

LGTM besides the changelog fragment.

changelogs/fragments/correct-classifiers.yaml Outdated Show resolved Hide resolved
'Programming Language :: Python :: 3.8',
{%- endif %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is OK as a first fix, but we should really make this depend on python_requires.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this'll work for now but it could be improved. We could get ansible-core's classifiers from the PyPI API and copy the ones that start with Programming Language :: Python :: 3 or something like that. (We could also get rid of they _python key in the deps files and retrieve ansible-core's python_requires from there.)

'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
{%- if version.major >= 7 %}
'Programming Language :: Python :: 3.11',
{%- endif %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should simply copy the values over from ansible-core? Might be easier than having specialized code which "magically" creates this.

@felixfontein felixfontein merged commit 1be2681 into ansible-community:main Dec 15, 2022
@felixfontein
Copy link
Collaborator

@gotmax23 thanks for fixing this!

@gotmax23
Copy link
Contributor Author

Thanks for reviewing

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.

2 participants