Skip to content

Docs. Remove helpers. #8647

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vbotka
Copy link
Contributor

@vbotka vbotka commented Jul 16, 2024

SUMMARY

Let's remove the helpers. Quoting from #8645

... dynamic way to generate documentations within the repository ... is not something we would like to do. There are processes that read (and expect) .rst files only within that directory.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

@ansibullbot
Copy link
Collaborator

@vbotka This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on Libera.chat IRC

click here for bot help

@ansibullbot ansibullbot added docs docs_only needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 16, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

At least lists_mergeby should be kept.

@vbotka
Copy link
Contributor Author

vbotka commented Jul 16, 2024

At least lists_mergeby should be kept.

This is up to you.

I have no resources to manage conformant versions within the repository. Please keep the helpers at your discretion if you don't accept this PR.

@russoz
Copy link
Collaborator

russoz commented Feb 16, 2025

@felixfontein I am revisiting this pending PR. It is not entirely clear to me how we should handle it. Back in the day I assumed we did not want to have dynamic code under the docs/docsite tree, hence we should remove the helper subdirs. It must be noticed that the code in helpers was used to generate content in docs/docsite/rst, and this is likely not used again for (potentially) a very long time.

Then you commented that "lists_mergeby should be kept". Are you referring to the code in helper? Because that is what this PR is about. The contents in rst is not touched by it.

I have mixed feelings about the entire idea: on one hand it might be interesting to have the docs showcasing actual test results, on the other hand, if we were to embrace that idea I think we might want to create a better, integrated, structure for this, potentially tapping on both unit and integration tests. For that, we would likely need to create standard structures/elements in the 3 subdirectories: docs/, tests/integration and tests/unit. I reckon that would be a slightly larger piece of work than just a single PR. Anyways, I digress.

Back to this PR, could you please clarify what you mean it should be kept? Are you actually referring to the playbooks and accompanying files in docs/docsite/helper/lists_mergeby ? Again, the RST files generated by those playbooks are already commited.

Appreciate your thoughts on the matter. TIA

@russoz
Copy link
Collaborator

russoz commented Feb 16, 2025

One additional comment here, after I tumbled deeper into the rabbit hole. There are these 3 distinct PRs related to this matter:

After reading comments in all three and my own rumblings in the previous comment here, I am leaning towards an idea of a solution that would look like this:

  • docs/docsite/helper would contain only data, no code at all,
  • the logic of generating docs from templates (part of that data) + tests and their outputs would not exist within this repository, rather it would be an external tool
    • that tool would provide standard mechanisms for dealing with the integration tests and the templates
    • that tool would perform validations on the structure and fail if any test or template is not compliant
    • that tool would, optionally, generate the files directly into the rst folder for committing, or elsewhere for debugging (though using rst in a separate branch should probably be enough).

Further automation and segregation of duties might be applied here, that's just a brain dump.

About this PR (and the others), I think we might be lenient and keep that working code in place, but we should block other plugins from creating new "helpers" in the same way.

@vbotka @felixfontein what do you think?

@vbotka
Copy link
Contributor Author

vbotka commented Feb 16, 2025

Still the same. Remove the helpers or keep them at your discretion and maintenance. Then, I'll take care of the docs.

@russoz
Copy link
Collaborator

russoz commented Feb 23, 2025

ping @felixfontein

@felixfontein
Copy link
Collaborator

After reading comments in all three and my own rumblings in the previous comment here, I am leaning towards an idea of a solution that would look like this:

* `docs/docsite/helper` would contain only data, **no code at all**,

That would be preferred. But, that requires code somewhere. What you describe next:

* the logic of generating docs from templates (part of that data) + tests and their outputs would not exist within this repository, rather it would be an **external tool**
  
  * that tool would provide standard mechanisms for dealing with the integration tests and the templates
  * that tool would perform validations on the structure and fail if any test or template is not compliant
  * that tool would, optionally, generate the files directly into the `rst` folder for committing, or elsewhere for debugging (though using `rst` in a separate branch should probably be enough).

would be my favorite solution, but right now we don't have such a tool, neither a generic one inside the repo, nor one outside. Until that happens, we should keep the helpers.

Generally, about the tool: I think it's somewhat comparable to Python doctests (https://docs.python.org/3/library/doctest.html). My idea would be that the tool allows you to provide some hidden setup code as comments in the RST files, some public code in there that defines what has to be run (together with the hidden setup code), and the output (resp. some part of it). The tool would have two modes: a) update the output, b) check the output (for CI, to point out that the output needs to be updated).

Further automation and segregation of duties might be applied here, that's just a brain dump.

CI should run the tool to make sure no updates are needed, and it should be somewhat easy to use (especially for not so experienced contributors).

Note that I tried out using docutils to find code blocks to lint YAML in them in ansible/ansible-documentation#2385, maybe something similar could be used to implement this. Or maybe the tool should operate on a lower level where it simply treats RST documents as a text file and uses pattern matching for finding the appropriate places. (Downside: if you put an example for this tool inside a code block, the tool will likely not notice that it's inside a code block and still process it...)

@vbotka
Copy link
Contributor Author

vbotka commented Feb 25, 2025

... right now we don't have such a tool, neither a generic one inside the repo, nor one outside. Until that happens, we should keep the helpers.

For the record. I don't maintain docs/docsite/helper/* anymore.

@russoz
Copy link
Collaborator

russoz commented Feb 26, 2025

Hi @gundalow, you asked before about how the development and tools team could help, this issue here might be the kickstart for something interesting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs_only docs needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants