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

Fix/improve version_added #34

Merged
merged 4 commits into from
Sep 14, 2022

Conversation

felixfontein
Copy link
Collaborator

Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

Looks good to me, for curiosity sake, do you happen have rendered output to see the differences?

@felixfontein
Copy link
Collaborator Author

Yes, see the above links :)

@felixfontein
Copy link
Collaborator Author

I wonder whether I should add special code to write ansible-base for 2.10 and Ansible for 2.9 and before. @briantist what do you think?

@briantist
Copy link
Contributor

Yes, see the above links :)

🤦‍♂️ in my defense, I am not usually up this early

I wonder whether I should add special code to write ansible-base for 2.10 and Ansible for 2.9 and before. @briantist what do you think?

It seems you just pushed the commit. I was going to say "nah don't bother since they're EoL" but I realize now that the version added stuff will stay as long as those things haven't since been updated, so I think it's nice. And since it's already done, no need to remove it.

@felixfontein
Copy link
Collaborator Author

@briantist I also updated the built docs, so if you're happy I'll merge :)

@briantist
Copy link
Contributor

Hm I have one possible English nit:

added in 0.5.0 of felixfontein.acme

To me it feels slightly unnatural, but it feels natural to say any of

added in version 0.5.0 of felixfontein.acme
added in v0.5.0 of felixfontein.acme
added in ver 0.5.0 of felixfontein.acme
added in felixfontein.acme version 0.5.0
added in felixfontein.acme v0.5.0
added in felixfontein.acme ver 0.5.0
added in felixfontein.acme 0.5.0

It's not a big enough thing for me to block over, and I'm also sensitive to screen real-estate for the text. Maybe get some other opinions on that.

@felixfontein
Copy link
Collaborator Author

How about added in felixfontein.acme 0.5.0? (Before it was writing added in 0.5.0 of, which definitely was much worse ;-) )

@felixfontein
Copy link
Collaborator Author

The docsite is now updated. See https://ansible.fontein.de/collections/community/crypto/crypto_info_module.html#ansible-collections-community-crypto-crypto-info-module for another example (module's version_added).

@felixfontein felixfontein merged commit 35ac5c6 into ansible-community:main Sep 14, 2022
@felixfontein felixfontein deleted the version_added branch September 14, 2022 13:10
@felixfontein
Copy link
Collaborator Author

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

2 participants