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

proxmox: deprecate old compatibility feature flag #6836

Merged
merged 3 commits into from
Jul 4, 2023
Merged

proxmox: deprecate old compatibility feature flag #6836

merged 3 commits into from
Jul 4, 2023

Conversation

russoz
Copy link
Collaborator

@russoz russoz commented Jul 3, 2023

SUMMARY

Deprecate parameter proxmox_default_behavior added in version 1.3.0.

ISSUE TYPE
  • Refactoring Pull Request
COMPONENT NAME

plugins/modules/proxmox.py

@ansibullbot
Copy link
Collaborator

cc @Ajpantuso @Thulium-Drake @UnderGreen @joshainglis @karmab @tleguern
click here for bot help

@ansibullbot ansibullbot added module module needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 3, 2023
@felixfontein
Copy link
Collaborator

How about bumping the deprecation to c.g 10.0.0? That gives users some more time to do the adjustment.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 labels Jul 3, 2023
@russoz
Copy link
Collaborator Author

russoz commented Jul 3, 2023

How about bumping the deprecation to c.g 10.0.0? That gives users some more time to do the adjustment.

That's starting to look like a norm, @felixfontein :)

Personally I think this has been around and documented for a long time. Users should/would not be using compatibility behaviour in a long time. Those who eventually are had to make that choice after reading the docs, so they should be aware.

For this case I would prefer to solve it sooner rather than later.

@felixfontein
Copy link
Collaborator

Removing options where there's no pressing need to remove them should IMO have a longer deprecation period. It's OK IMO to warn them about the upcoming deprecation, complete removal is another level. If you would only deprecate and remove the compatibility value of this option I think 9.0.0 is fine. But it could be that quite some users explicitly added this option explicitly with value no_defaults some time ago, and now they are 'punished' for doing that by giving them a short timeframe to remove it again.

@russoz
Copy link
Collaborator Author

russoz commented Jul 3, 2023

IMO 1+ year is not a "short timeframe", but not pressing the point. Bumping up to 10.0.0.

@felixfontein felixfontein merged commit c34a227 into ansible-collections:main Jul 4, 2023
@patchback
Copy link

patchback bot commented Jul 4, 2023 •
edited
Loading

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/c34a22717e40a532529e0283c973881f4f8530b6/pr-6836

Backported as #6851

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jul 4, 2023
patchback bot pushed a commit that referenced this pull request Jul 4, 2023
@russoz @patchback
proxmox: deprecate old compatibility feature flag (#6836)
3497174 
* proxmox: deprecate old compatibility feature flag

* add changelog frag

* bump deprecation to 10

(cherry picked from commit c34a227)
@patchback patchback bot mentioned this pull request Jul 4, 2023
Merged
@felixfontein
Copy link
Collaborator

@russoz @UnderGreen thanks!

felixfontein pushed a commit that referenced this pull request Jul 4, 2023
@patchback @russoz
[PR #6836/c34a2271 backport][stable-7] proxmox: deprecate old compati…
1c8fbed 
…bility feature flag (#6851)

proxmox: deprecate old compatibility feature flag (#6836)

* proxmox: deprecate old compatibility feature flag

* add changelog frag

* bump deprecation to 10

(cherry picked from commit c34a227)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
@russoz russoz deleted the proxmox-deprecate-old-compatibility branch July 4, 2023 19:41
@felixfontein felixfontein mentioned this pull request Aug 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@UnderGreen UnderGreen UnderGreen approved these changes

@felixfontein felixfontein felixfontein approved these changes

Assignees
No one assigned
Labels
module module plugins plugin (any type)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@russoz @ansibullbot @felixfontein @UnderGreen