-
Notifications
You must be signed in to change notification settings - Fork 11
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
Deprecating the old module vm_list_group_by_clusters #25
Deprecating the old module vm_list_group_by_clusters #25
Conversation
423c860
to
9687734
Compare
major_changes: | ||
minor_changes: | ||
- Added module vm_list_group_by_clusters |
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'm not sure if it's acceptable to alter the changelog after the release. Not 100% sure, though.
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'm not sure either but that's was my understanding from his comment in that discussion
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 it's OK, though there's another file https://github.com/ansible-collections/vmware.vmware/blob/main/CHANGELOG.rst where the changes should be reflected.
@felixfontein if we change the entries in both the files same way, will ansibull-changelog be OK with it?
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.
Thanks I updated that file as well
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.
CHANGELOG.rst is likely autogenerated, you can use antsibull-changelog generate
to update it from changelogs/changelog.yaml
.
It's best to not manually edit the output file since antsibull-changelog will overwrite it for the next release anyway. Better only change the YAML file (either directly or indirectly through antsibull-changelog release
) and let antsibull-changelog update the output files.
(You can also enable MarkDown output for antsibull-changelog BTW.)
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.
didn't know such a feature exists:), thanks!
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.
Thanks @felixfontein I reverted the manual change and ran antsibull-changelog generate
and pushed it.
Just wanted to work on this, also 😆 |
@mariolenz thank you for your comments :) |
37d2a90
to
9128834
Compare
9128834
to
6d75d8c
Compare
6d75d8c
to
26253ea
Compare
@mariolenz @felixfontein @Andersson007 Any disclaimers regarding that 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.
LGTM
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.
One last detail- changelog fragments are using ReStructured Text, so you need double backticks.
changelogs/fragments/25-deprecated_module_vm_list_goup_by_clusters.yml
Outdated
Show resolved
Hide resolved
…ters.yml Co-authored-by: Felix Fontein <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
===========================================
+ Coverage 13.71% 29.58% +15.86%
===========================================
Files 7 8 +1
Lines 1917 605 -1312
Branches 478 112 -366
===========================================
- Hits 263 179 -84
+ Misses 1654 426 -1228
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
thank you @felixfontein |
SUMMARY
Deprecating the module vm_list_group_by_clusters since it will be replaced with vm_list_group_by_clusters_info in v2.0.0
ISSUE TYPE
Adding a deprecating
COMPONENT NAME
Deprecating the module vm_list_group_by_clusters since it will be replaced with vm_list_group_by_clusters_info in v2.0.0 and since then we will have 2 modules.
I used that doc in order to sign that module as deprecated