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

x509_crl: prepare releasing the mode option for AnsibleModule's use #596

Merged
merged 2 commits into from
Apr 29, 2023

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Since ansible/ansible#80449 got merged the module no longer works with ansible-core devel, since the mode values generate and update are not allowed by ansible-core's validation.

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

x509_crl

@felixfontein
Copy link
Contributor Author

@felixfontein
Copy link
Contributor Author

Restart CI.

@github-actions
Copy link

github-actions bot commented Apr 28, 2023

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.crypto/branch/main

version_added: 2.13.0
mode:
description:
- This parameter is deprecated and will be removed in community.crypto 3.0.0. Use I(crl_mode) instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this option is the new crl_mode option, as defined in line 45, but it's being set at deprecated with a message to use crl_mode "instead". Am I misreading something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes; basically I renamed mode to crl_mode, but instead of just adding mode as an alias to crl_mode I have to keep it as a separate option, since otherwise mode as an option would show up from AnsibleModule's add_file_common_args=True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about changing the documentation to:

Suggested change
- This parameter is deprecated and will be removed in community.crypto 3.0.0. Use I(crl_mode) instead.
- This parameter has been renamed to I(crl_mode). The old name I(mode) is now deprecated and will be removed in community.crypto 3.0.0.
Simply replace usage of this parameter with I(crl_mode).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see my mistake, I completely missed line 59. What had me confused was thinking that 61 was still part of crl_mode 🤦

With that, the old description was fine. The new description is good too; only thing I'd avoid is use of the word "simply" but maybe that's my own feeling only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed 'simply', see 6666baf.

@felixfontein felixfontein merged commit c568923 into ansible-collections:main Apr 29, 2023
127 checks passed
@felixfontein felixfontein deleted the x509_crl-mode branch April 29, 2023 18:54
@felixfontein
Copy link
Contributor Author

@briantist 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.

None yet

2 participants