-
Notifications
You must be signed in to change notification settings - Fork 200
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
Drop Foreman < 3.8 versions support #1844
Conversation
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.
I think this current PR should be summarized as "Update forklift_versions role tests to use current versions" or similar.
But taking a step back, I'd combine this with another change: drop older boxes. We have versions.yaml and we can drop older versions there.
What we at least need to keep is currently supported versions (3.10, 3.11 & nightly) and what's needed to release them. To release 3.10, we need n-2 so 3.8. That means you can drop 3.7 and older.
To do that, you need to update the tests, but that's usually not the objective. So the PR summary should then be something like "Drop Foreman < 3.8 versions".
When dropping older version support, you can take a look if there are some version specific hacks that can also be dropped, but there is some downstream concern. Forklift is also used in Satellite so you may need to keep 3.5 support for now.
You can consider dropping the versions 1 by 1, each in a separate commit. Start with the oldest and work your way to the newest.
@ekohl I've been using this file to check the versions compatibility but how can I review the dependency for |
3418ffc
to
9a9a180
Compare
9a9a180
to
6d89c6f
Compare
400dc9d
to
6c02adc
Compare
0c437ab
to
1161d1f
Compare
d0bdd41
to
2a3cba2
Compare
2edff7d
to
1b93f6c
Compare
1b93f6c
to
7e6fd50
Compare
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.
Was dropping centos 7 & 8 intended to be part of this PR?
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.
I thought it's fine as a cleanup? :)
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.
I'd have preferred it to be more explicit and at least as a separate commit.
#1826