[RFC] Updating the README generator automation

[sbidoul]

[RFC] Updating the README generator automation

[sbidoul]

With #566 (which was not that hard to implement after all :), I think pre-commit becomes viable.

We now need to find a way to coordinate this pre-commit deployment with the disabling of the README generator in the bot.

[fcvalgar]

There are ongoing discussions with the functional working groups about converting the readme fragments to markdown for easier authoring by non technical folks. So I'm putting this topic on pause at the moment.

Hi @sbidoul how can participate in the functional groups? Thank you.

[sbidoul]

how can participate in the functional groups

@fcvalgar you'll find more information about that in the last OCA newsletter.

[TumbaoJu]

I still don't understand the benefits of using Markdown over RST. Both requires to learn their syntax, and MD it's not simpler than RST IMO. But switching to MD will have a barrier to render things in Odoo, and HTML is not the solution to have them displayed.

@pedrobaeza : The main reason for using Markdown, apart from it's very simple syntax to learn and available editor , is to be able to drag & drop images into the file. With rst files, we have to download the image in one place, then add a link in the documentation to the image and if you make a mistake in the link, the image won't show.

It's easier to understand how to use the module when the Read Me contains screenshots.

In the project of including / attracting more functionals into contributing to OCA, modules documentation is a great way to use their skills. We, functionals, usually, love to write documentation!

I hope this explanation helps you understand where this idea comes from and why we want to do this change.

[pedrobaeza]

Hi, Julie, thanks for the extra clarification. I understand now better the motivations, and indeed that's the only clear difference between MD and RST, and it's not due to language syntax itself, but the feature GitHub offers, which includes image drag and drop. I see that they are uploaded in the repository in assets subfolder (example: https://github.com/OCA/maintainer-tools/assets/7165771/f4297003-f18d-4b4d-b75f-36a0d4d88ada). Do we know if such "assets" have any expiration date?

[pedrobaeza]

Yes, let's improve existing workflow meanwhile.

[sbidoul]

The OCA GitHub bot has now been updated with the latest readme generator. So closing this.

[pedrobaeza]

Can't we freeze the versions in pre-commit config for being sure no change is done?

[sbidoul]

Can't we freeze the versions in pre-commit config for being sure no change is done?

Of course but it is not different from the current situation where the version is frozen in the bot.
The problem occurs when we want to upgrade the readme generator and we don't want to update 12 000 addons.

[pedrobaeza]

But we control that with the copier template. We can apply such update only for the newest version, and reduce the impact. We are already doing this for black IIRC.

[yajo]

It seems to me that the format of the README.rst is more or less stable, whereas the format of the static/description/index.html is more prone to changes depending on dependency versions. That is because README.rst just gets generated with a dumb jinja template, but index.html goes through a full rendering pipeline.

One simple guard would be to instruct oca-gen-addon-readme to not regenerate index.html if README.rst didn't change. Most times it won't change, so it should be just OK to leave it as a pre-commit hook and let the bot forget about readmes.

IIUC the bot is the same for all version branches. Removing the readme generation from the bot would free it from having to pin the maintainer-tools version.

1. Doing it in pre-commit and not with the bot

[...] each update to the repo template would trigger a massive re-generation

I think this is not really a problem because of what @pedrobaeza said. In the template, we already have a system to pin hooks per python version. You can see that maintainer-tools is at a different point in history per branch.

In OCA/oca-addons-repo-template#190 I proposed to reuse that version, but we could use a different version for just the oca-gen-bot, which could be the same as the one currently used by the bot. There's no problem on that.

2. Re-generating the README only when the content of the readme/ directory changes

This wouldn't work for pre-commit. And if we don't use pre-commit, we have the problem from OCA/oca-addons-repo-template#190 (comment). If we're gonna change, let's change it for pre-commit if possible. That way, we will not only fix the problem for maintainers, but for functional reviewers too.

3. Generating the README upon ocabot merge.

This is what we have currently, right?

[sbidoul]

Code autofixers and autoformatters are pinned per branch and we generally have no reason to change them, so once we have chosen a version and configuration, they will not change the code.

The problem is that 2 or 3 times a year, people have ideas to improve the readme generator which alter the rst output.
If we use pre-commit and apply those to the repo template it triggers a massive re-generation.

[sbidoul]

Actually option 2 may work with pre-commit too: the idea in #520 is to store a comment in README.rst with a hash of the content of the readme/ directory. As long as the content of the README matches the hash, nothing is re-generated.

[pedrobaeza]

I think the speed of such proposals is now slower (and the last ones have been for adapting this tool to non-OCA flows), but anyway, it's reasonable to pin the generator to a fixed version on each branch, and only bump it for the following version, not needing all these checksum mechanisms, as the output will be still the same.

If you are thinking on the bot having only one version for all the branches, the answer is simple: the bot should disappear in this case, as pre-commit will handle it.

[pedrobaeza]

You tell us 😉

[yajo]

I see the fundamentals are merged now. How should we proceed?

[sbidoul]

There are ongoing discussions with the functional working groups about converting the readme fragments to markdown for easier authoring by non technical folks. So I'm putting this topic on pause at the moment.

[yajo]

That'd be great indeed. I think the problem is that Odoo doesn't support markdown, but nevertheless if we produce the index.html file, it will ignore the README anyway, so that problem could be avoided.

[sbidoul]

Yes my current idea is to generate an index.html that Odoo will use in priority for display in the apps list, and a README.md (used for GitHub preview and PyPI but ignored by Odoo).

[yajo]

Well, some random thoughts so you can keep those in your internal discussions:

• I don't know others, but our functional people generally look at the README by searching the module within the runboat instance of the PR. They don't dig into the code, and we don't expect them to do so.
• When looking for the module, they use the search function from https://odoo-community.org/shop which has READMEs rendered.
• GitHub the README.rst files fine enough for most cases.
• If we start generating a README.md, would that mean that all readme/*.rst files should be converted to readme/*.md? Isn't that too much burden on devs?
• If we plan to use something like pandoc to convert readme/*.rst into a README.md, isn't that too complex?
• Maybe we could add the .md support without dropping the .rst to avoid those last 2 problems?
• Should we progress with OCA/oca-addons-repo-template#190 as an MVP to fix that problem, while this other subject is discussed and analyzed?

Thanks for your efforts on this subject!

[fcvalgar]

how to participate in the functional groups > > @fcvalgar You can find more information about this in the [latest OCA newsletter] (https://odoo-community.org/blog/news-updates-1/oca-newsletter-1-2023-136).

Thank you @sbidoul . The OCA website dont work. I will come back later to review it.

[pedrobaeza]

I don't think we should switch to Markdown and make all the OCA contributors to learn the new syntax. Both are simple enough for other people.

Anyway, we should go on with the README generator in pre-commit, and if later needs to be changed to other syntax, it can be done, but let's not block this good improvement due to this.

[sbidoul]

I'm sill pondering this.

It occurred to me that there is an important drawback to the pre-commit approach. Non-technical contributors updating documentation using GitHub will not be able to do so if pre-commit is rejecting their PR because it updates the README file from the modified fragments.

[pedrobaeza]

Although true, I think that's not the main problem, as current GitHub tools is not allowing to edit more than one file in one shot. Anyway, we can merge such PRs "in red" with the merge button, and let the BOT to regenerate the README on merge to have again a green CI. Or explore the option to only activate that check on git commit, but not on the CI test.

[yajo]

What is more common?

1. A functional person reviewing a PR made from a technician. The PR includes README changes, which don't appear in the module description while the functional person is reviewing in the runboat. This makes the functional review confusing.
2. A functional person updating a README.rst file, using appropriate RST syntax.

FWIW I see case 1 happen a lot, while I can't remember having seen case 2 yet.

[sbidoul]

current GitHub tools is not allowing to edit more than one file in one shot

This is now possible with github.dev where you get an in-browser vscode instance. With a little tutorial, this is a fine environment where non technical folks can author documentation (including markdown preview) without installing anything on their local machine.

What is more common?

Today we don't have many functional people contributing documentation, that is true. One reason that they identified is that it is too difficult. But it is a strategic objective of the OCA to make it easier for non-technical people to engage with the community. And providing them an easier way to author documentation is one way that has been identified to achieve that objective.

So we need to come up with a solution that does not make this objective harder to reach.

A functional person updating a README.rst file, using appropriate RST syntax.

That is why there is this proposal above to allow using markdown fragments. I think the conversion is quite doable with pypandoc. I have a protoype that I'll post as a PR soon.

[pedrobaeza]

I still don't understand the benefits of using Markdown over RST. Both requires to learn their syntax, and MD it's not simpler than RST IMO. But switching to MD will have a barrier to render things in Odoo, and HTML is not the solution to have them displayed.

[yajo]

FWIW I do think markdown is way more simple than rst, in many regards. So I'm not against that at all! The drag and drop, the ubiquity or the language and the availability of WYSIWYG editors just add value.

However I still think we're mixing 2 things here. The current proposal is IMHO more suited as an MVP to improve current situation: a dev writing a readme, and a functional person not being able to read it. It implies to flip a couple of simple switches:

1. Tell bot to add --if-fragments-changed
2. Add a pre-commit hook with that option too, into the template

Your proposal is totally different: change our documentation workflow. It implies:

• Visual editor.
• New tutorials.
• Language change (rst -> md).
• New tooling (README.rst -> README.md -> static/description/index.html)
• What about videos?
• Do assets expire?
• What about translations?
• What about a browsable translations site?
• What about using the wiki?
• Will we render docs somehow? Mkdocs, readthedocs, etc.?
• etc.

I don't think it's a good idea to block this MVP just because there's another ongoing refactor on a higher level.

[sbidoul]

@yajo, markdown support is independent of the pre-commit vs bot discussion for README generation.

The reasons why the pre-commit approach does not work in its current proposed form are summarized in OCA/oca-addons-repo-template#190 (comment).

So for the moment I think I'll move forward with option 2 later this summer.