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

Deprecation by date #62

Merged
merged 18 commits into from
Jun 23, 2020

Conversation

felixfontein
Copy link
Collaborator

No description provided.

@felixfontein felixfontein requested a review from abadger June 1, 2020 14:56
@abadger
Copy link
Contributor

abadger commented Jun 1, 2020

My first instinct on seeing this was that it belongs in the schema because it is data manipulation, not just display. It's going to be desirable for Galaxy and other things that need to display the documentation to have access to this as well.

But thinking about how it would fit into the schema, i think it goes a little beyond what the schema is responsible for. It's creating data rather than normalizing existing data.

So i think this belongs here:

https://github.com/ansible-community/antsibull/blob/master/antsibull/cli/doc_commands/stable.py#L256

There needs to be a function which calculates additional data from what was actually pulled out of the documentation fields. For instance, full_path for outputting the suboptions correctly will end up in that function. I think this is similar and belongs in that function as well

@abadger
Copy link
Contributor

abadger commented Jun 1, 2020

@awcrosby I think this new function that I mentioned to @felixfontein above is what you were wanting as well -- a place where we can have common code that converts or adds to the raw documentation data structures so all of us generating output from it have an easier time. Maybe you'll want to hit me up on slack (or jump on irc, #ansible-docs) to talk more about whether that can work for you too?

@abadger abadger added enhancement New feature or request Priority Priority to implement this labels Jun 7, 2020
@felixfontein felixfontein force-pushed the deprecation-by-date branch from 45d6e72 to beb10fe Compare June 16, 2020 05:53
@felixfontein
Copy link
Collaborator Author

I completely restarted this branch from scratch. It now contains the schema definitions currently in use in Ansible for deprecation by date, and for (auto-)adding collection names.

@felixfontein felixfontein force-pushed the deprecation-by-date branch from beb10fe to df9da48 Compare June 16, 2020 06:05
…ow option deprecation for module options. (These are deprecated by argument spec.)
@felixfontein
Copy link
Collaborator Author

I've split up DeprecationSchema into TopLevelDeprecationSchema (uses removed_in, removed_at_date and removed_from_collection) and OptionDeprecationSchema (uses version, date and collection_name), and made sure that only plugin options can be deprecated this way, but not module options. (Module options are deprecated via argument spec.)

This makes the schema be compatible to what Ansible's code and validate-module's schema currently require resp. enforce.

The top-level name -> option name conversions in OptionDeprecationSchema are debatable, since Ansible itself will only accept the option name fields. Maybe we should remove the conversions?

@felixfontein
Copy link
Collaborator Author

felixfontein commented Jun 16, 2020

@abadger I'm confused. Where does that "version_added": "historical" come from in all the good schema validation data? I don't think ansible-doc exports that. Ignore that, I think I understood what's going on :)

@felixfontein felixfontein requested a review from abadger June 16, 2020 07:12
@felixfontein
Copy link
Collaborator Author

felixfontein commented Jun 16, 2020

(What's missing right now is a XOR check for deprecation by date and deprecation by version.) Added.

@abadger abadger changed the base branch from master to main June 19, 2020 04:18
antsibull/write_docs.py Outdated Show resolved Hide resolved
@abadger
Copy link
Contributor

abadger commented Jun 23, 2020

Looks good, thanks!

@felixfontein felixfontein merged commit 81bd28d into ansible-community:main Jun 23, 2020
@felixfontein felixfontein deleted the deprecation-by-date branch June 23, 2020 06:00
@felixfontein
Copy link
Collaborator Author

@abadger also thanks a lot for reviewing this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Priority Priority to implement this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants