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

Remove Python version check #167

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

felixfontein
Copy link
Collaborator

It was checking for Python 3.6, for which we dropped support quite some time ago (we're requiring Python 3.9+ since 2022-12-17).

@felixfontein felixfontein requested a review from samccann June 3, 2024 17:53
Copy link
Contributor

@samccann samccann left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gotmax23 gotmax23 left a comment

Choose a reason for hiding this comment

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

I would just remove this entirely. We already set requires_python in the metadata.

@samccann
Copy link
Contributor

samccann commented Jun 4, 2024

Hmm. I can see the value of only maintaining the version in one location in source, but from a user perspective, if I just do a 'pip install antsibull-changelog' on a system with python 3.8 for example, it doesn't give me any errors. So if we remove the version check in this PR, how will I know it's not working (and why?).

@felixfontein
Copy link
Collaborator Author

@samccann you'll likely get an older version then that still works with Python 3.8 :) Only with extremely old pip versions that didn't check for the Python version would you get the latest version, but then the Python is probably also very old and it would complain about syntax errors before running any code :)

@felixfontein felixfontein changed the title Adjust Python version check Remove Python version check Jun 4, 2024
@felixfontein
Copy link
Collaborator Author

I rewrote the PR to remove the check.

Copy link
Contributor

@samccann samccann left a comment

Choose a reason for hiding this comment

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

lgtm

@felixfontein felixfontein merged commit b8e9b42 into ansible-community:main Jun 5, 2024
5 checks passed
@felixfontein felixfontein deleted the python branch June 5, 2024 05:33
@felixfontein
Copy link
Collaborator Author

@samccann @gotmax23 thanks a lot for reviewing this!

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