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

consolidate argument specs and doc fragments #89

Conversation

mikemorency
Copy link
Collaborator

@mikemorency mikemorency commented Dec 11, 2024

SUMMARY

Combining #86 and #87

This removes instances of repeated option documentation and redundant spec args.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

doc fragments
argument specs

ADDITIONAL INFORMATION

While I was working on this, I found the kubernetes collection has a pretty nice pattern for breaking out the doc fragments. I tried to copy theirs, https://github.com/ansible-collections/kubernetes.core/tree/main/plugins/doc_fragments
Similar pattern for the arg specs too https://github.com/ansible-collections/kubernetes.core/blob/main/plugins/module_utils/args_common.py

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@mikemorency mikemorency force-pushed the feature/consolidate-arg-specs-and-docs branch from 9d32d91 to 6a60822 Compare December 11, 2024 02:06
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.97%. Comparing base (94ea806) to head (c09f40f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
+ Coverage   27.48%   27.97%   +0.48%     
==========================================
  Files          24       25       +1     
  Lines        2059     2073      +14     
  Branches      385      384       -1     
==========================================
+ Hits          566      580      +14     
  Misses       1493     1493              
Flag Coverage Δ
sanity 27.97% <100.00%> (+0.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@anna-savina anna-savina left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

@mikemorency I'd like to test your changes with community.vmware. That is, what would happen if I try to (re-) use them there.

Please give me a few days for this.

@mikemorency
Copy link
Collaborator Author

Sure thing @mariolenz, I appreciate the additional testing. If possible, I'd like to merge by the 18th so we can do some additional QA before the targeted release on the 23rd

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

@mikemorency Apart from my last comment above LGTM

ref ansible-collections/community.vmware#2273

@mikemorency mikemorency merged commit fe776ce into ansible-collections:main Dec 16, 2024
13 checks passed
@mikemorency mikemorency deleted the feature/consolidate-arg-specs-and-docs branch December 16, 2024 13:50
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

3 participants