-
Notifications
You must be signed in to change notification settings - Fork 52
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
Fix deprecated code #677
Fix deprecated code #677
Conversation
6f8a318
to
dfc2087
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.
We don't use upstream's deprecation/removal policy and can't remove features in a maintenance update
@agraul I get that but the code doesn't work right now - we're including it, but if a user touches the code, it'll blow up with an exception. Isn't it the same state as right now? Would you prefer that we diverge from upstream and modify the dates? |
From a quick look, I think we don't know if it's broken. Salt's deprecation mechanism Salt uses this to force their developers to stick to the deprecation timeline and actually remove code once it reaches that date. For us it's annoying, this isn't the first time it has happened, because we have longer support cycles and don't want to drop features without moving to a new major version. |
Due to SUSE's extended support policy, we won't remove code from Salt until next major release.
dfc2087
to
6ac62f8
Compare
@agraul I take it from your response that you would prefer for us to diverge from upstream, even if the code was blowing up until now (and therefore, should be safe to remove since we had no complaints). In such case, this should be good enough. I set the date to Question: shall I also adjust https://github.com/openSUSE/salt/blob/openSUSE/release/3006.0/salt/modules/aptpkg.py#L3130 ? We adjusted the date to 2025, but I'm quite sure we won't update to 3007.x by Jan 1st, 2025, right? |
Yes. When it comes to the support lifecycle, we are not following upstream. Again, we can't drop features in minor updates. I understand that this feature was broken for months, but now that we know about it, we should fix it according to our lifecycle policy.
We won't move to the 3007 series, that's a short-term-support release from upstream. They won't support it for as long as they will support 3006 and 3008, by skipping it we can benefit from reduced backporting efforts. 2026 should be okay.
We should start packaging 3008 soon, but we won't replace 3006 by January, that's correct. We should either bump this date, or change |
Ack, makes sense.
Personally, I'd prefer to adjust the date as that change is simpler (both to understand and to implement) than modifying the |
Due to SUSE's extended support policy, we won't remove code from Salt until next major release.
What does this PR do?
This PR backports saltstack/salt@67b08abThis PR fixes runtime errors connected to
warn_until_date
code that's been deprecated upstream. This PR works around the code removal in saltstack/salt@67b08abWhat issues does this PR fix or reference?
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1226118
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.