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.

Sign up for GitHub

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

Jump to bottom

remove support for building ansible < 6.0.0 #477

Merged
merged 3 commits into from
May 5, 2023
Merged

remove support for building ansible < 6.0.0 #477

merged 3 commits into from
May 5, 2023

Conversation

gotmax23
Copy link
Contributor

@gotmax23 gotmax23 commented Dec 10, 2022 •
edited
Loading

Closes: #472

@gotmax23
Copy link
Contributor Author

Hmm, looks like ansible-community/antsibull-changelog@15b5bb0 broke the CI

@felixfontein
Copy link
Collaborator

Why should only the setup.py template be updated, and not everything else related to older Ansible versions?

Also please move the real bugfix (what you declare a 'minor change') to a separate PR, so it is separate from the breaking change (what you call 'bugfix').

@gotmax23
Copy link
Contributor Author

Why should only the setup.py template be updated, and not everything else related to older Ansible versions?

This was one glaring example that I noticed.

Also please move the real bugfix (what you declare a 'minor change') to a separate PR, so it is separate from the breaking change (what you call 'bugfix').

Yeah, not sure why I classified that as a bugfix... I'll fix that and split this out.

@gotmax23 gotmax23 force-pushed the setup.py branch 3 times, most recently from ac19497 to 05c5abf Compare December 10, 2022 22:39
felixfontein
felixfontein reviewed Dec 12, 2022
View reviewed changes
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.

Besides that, LGTM. We shouldn't merge this anytime soon, though, similar to #465 an #466. There should be at least one more bugfix release with fixed Programming Language :: Python :: classification generation code before we start merging breaking changes.

changelogs/fragments/477-setup_py.yaml Outdated Show resolved Hide resolved
@gotmax23 gotmax23 marked this pull request as draft December 14, 2022 20:27
@gotmax23
Copy link
Contributor Author

Marking this as a draft to prevent accidental merging

@gotmax23 gotmax23 mentioned this pull request Dec 14, 2022
Merged
@gotmax23 gotmax23 changed the title ansible setup.py: Remove code for old versions remove support for building ansible < 6.0.0 Apr 19, 2023
@gotmax23 gotmax23 force-pushed the setup.py branch 2 times, most recently from 16cde92 to d24ba10 Compare April 19, 2023 00:06
@gotmax23 gotmax23 mentioned this pull request Apr 19, 2023
Closed
felixfontein
felixfontein reviewed Apr 19, 2023
View reviewed changes
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.

We should probably fail if ansible_version < 6.0.0 is true somewhere early on, and mention that Ansible < 6.0.0 is no longer supported by this version. Potentially already do this in src/antsibull/cli/antsibull_build.py?

@felixfontein felixfontein mentioned this pull request May 4, 2023
Merged
@gotmax23
Copy link
Contributor Author

gotmax23 commented May 4, 2023

Should we allow setting some sort of env var (such as ANTSIBULL_BUILD_UNSUPPORTED_ANSIBLE_VERSION) to ignore the error or just fail completely?

@felixfontein
Copy link
Collaborator

I would simply fail completely. If someone wants to build an older version of Ansible, they should install an older antsibull version. And if for some reason such a workaround is needed in the future, it can still be added.

@gotmax23 gotmax23 force-pushed the setup.py branch 2 times, most recently from 046aa20 to bf7f315 Compare May 5, 2023 15:53
@gotmax23 gotmax23 marked this pull request as ready for review May 5, 2023 16:04
@gotmax23
Copy link
Contributor Author

gotmax23 commented May 5, 2023

We should probably fail if ansible_version < 6.0.0 is true somewhere early on, and mention that Ansible < 6.0.0 is no longer supported by this version. Potentially already do this in src/antsibull/cli/antsibull_build.py?

Done!

felixfontein
felixfontein reviewed May 5, 2023
View reviewed changes
src/antsibull/constants.py Outdated Show resolved Hide resolved
changelogs/fragments/477-setup_py.yaml Show resolved Hide resolved
@felixfontein
Copy link
Collaborator

There is also some more code in src/antsibull/build_changelog.py that can be removed; I can do that in a follow-up PR.

@felixfontein felixfontein merged commit fe1f294 into ansible-community:main May 5, 2023
5 checks passed
@felixfontein
Copy link
Collaborator

@gotmax23 thanks for doing this!

@felixfontein felixfontein mentioned this pull request May 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein approved these changes

Assignees
No one assigned
Labels
breaking_changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop code for old ansible versions
2 participants
@gotmax23 @felixfontein