Bot not converting readme fragments to markdown in post-merge

[SirAionTech]

Describe the bug

After a PR is merged, the bot does some post-merge changes, but the README regeneration is skipped, for instance see OCA/l10n-italy@e4e5192, where the pre-commit run says:

> Run pre-commit run --all-files --show-diff-on-failure --color=always
pre-commit run --all-files --show-diff-on-failure --color=always
  shell: /usr/bin/bash -e {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.11.9/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.9/x64/lib
    PY: ca87a0d061b6[2](https://github.com/OCA/l10n-italy/actions/runs/9807706147/job/27081983177#step:7:2)5c7086cb19f5a91f62afe3f5396bf8ede3a6cf7d7f1beb2bfde
    SKIP: oca-gen-addon-readme

(from https://github.com/OCA/l10n-italy/actions/runs/9807706147/job/27081983177#step:7:8).

Here something in the README gets updated, but it should also rename from .rst to .md the fragments in https://github.com/OCA/l10n-italy/tree/e4e5192952d8dc259f92668a3c397a43c1f708a8/l10n_it_financial_statement_eu/readme

Now the problem is that the oca-gen-addon-readme hook in local pre-commit runs fails because it tries to rename the mentioned fragments.

To Reproduce

Steps to reproduce the behavior:

1. Have a repo accepting .rst fragments.
2. Create a PR (this happened in [16.0][MIG] l10n_it_financial_statement_eu: Migration to 16.0 l10n-italy#3599) to add a module (it uses .rst fragments): the developer generates the README correctly.
3. Update the repo configuration to use .md fragments.
4. Merge the PR.

Expected
post-merge changes convert the README

Actual
post-merge does not convert the README
side-effect: all the developers working on the repo will have pre-commit failing and trying to add fragments conversion.

[sbidoul]

The bot is not expected to convert the fragments. That's the role of pre-commit or the developer.

[SirAionTech]

Thanks for having a look!

The bot is not expected to convert the fragments. That's the role of pre-commit or the developer.

Then the bot never regenerates the README?
Because the linked post-merge commit (OCA/l10n-italy@e4e5192), and also translation ones (like OCA/l10n-italy@0536eb0) all have

SKIP: oca-gen-addon-readme

I see that in the pull request CI (https://github.com/OCA/l10n-italy/blob/79f19bb8345fd5c1d04ea483b25f112269ba36fc/.github/workflows/pre-commit.yml#L38) the oca-gen-addon-readme is skipped too.

Just looking at the configuration, it looks to me like if someone pushes a change in a README fragment without regenerating the README, the README won't be checked or updated automatically, so the regeneration is something the next developer will have to do; what am I missing?

---

If this is more of a repo configuration issue, feel free to move it to the https://github.com/OCA/oca-addons-repo-template repo, I opened it here because I thought the bot was misbehaving.

[legalsylvain]

Just looking at the configuration, it looks to me like if someone pushes a change in a README fragment without regenerating the README, the README won't be checked or updated automatically, so the regeneration is something the next developer will have to do; what am I missing?

If yes, that's a regression. Before, the bot when merging was updating main README.rst file.

[SirAionTech]

Just looking at the configuration, it looks to me like if someone pushes a change in a README fragment without regenerating the README, the README won't be checked or updated automatically, so the regeneration is something the next developer will have to do; what am I missing?

If yes, that's a regression. Before, the bot when merging was updating main README.rst file.

Now I see

oca-github-bot/src/oca_github_bot/tasks/main_branch_bot.py

Line 37 in 2376d30

"--if-source-changed",

then it shoudn't be the case.

But still, I expected the README to be regenerated in OCA/l10n-italy@e4e5192.
Maybe it didn't happen because the PR it came from (OCA/l10n-italy#3599) was still using a pre-commit configuration that used .rst fragments and the bot is working on that branch?

[sbidoul]

I don't think there is any regression.

1. pre-commit, when run manually by the developper, does generate README.rst
2. the bot does generate README.rst when merging, and nightly, as you showed in OCA/l10n-italy@e4e5192
3. pre-commit in CI does not attempt to generate/check README.rst, for the reason explained in the comment
4. markdown conversion is indeed configured on l10n-italy 16.0, but because of 3/, it happens only when pre-commit is run manually by the developer, and if not done by the developer, this is not checked by CI

I understand you expect markdown conversion to be done automatically by the bot? But that's not how it is designed today. And honestly I don't think it would be a good thing to do.

[SirAionTech]

I am beginning to understand the steps to have this behavior:

1. Have a repo accepting .rst fragments.
2. Create a PR to add a module (it uses .rst fragments): the developer generates the README correctly.
3. Update the repo configuration to use .md fragments.
4. Merge the PR.

Expected
post-merge changes convert the README

Actual
post-merge does not convert the README
side-effect: all the developers working on the repo will have pre-commit failing and trying to add fragments conversion.

As you said, this is how it's designed now, but the side-effect is pretty annoying for any developer working on the repo.

[legalsylvain]

I don't think there is any regression.

thanks for the explanation !

[sbidoul]

As you said, this is how it's designed now, but the side-effect is pretty annoying for any developer working on the repo.

It's a complicated topic. What we have today is a trade-off between developer comfort and ease of contribution to fragments by functional folks, who want to do on GitHub directly without using pre-commit.

[SirAionTech]

As you said, this is how it's designed now, but the side-effect is pretty annoying for any developer working on the repo.

It's a complicated topic. What we have today is a trade-off between developer comfort and ease of contribution to fragments by functional folks, who want to do on GitHub directly without using pre-commit.

Yes, and I understand that the CI checks are relaxed because of that.
But I expected the bot to do anything a developer would, including converting the README; I understand it might be risky but without it the developers have one more burden.

Anyway, I'm fixing it in the repo and hope it won't happen too often.

[sbidoul]

Yes, the bot could do the markdown conversion but

1. its a somewhat risky operation that I think warrants manual review
2. it would require work to implement
3. I don't think it will happen frequently (for instance, the standard migration instructions require to run pre-commit which will do the conversion, and once the conversion is done, well it's done)

[SirAionTech]

@sbidoul could you please reopen this? I can't

[sbidoul]

I reopened. It does not change my opinion that we should not do this, though ;)