Skip to content

[REM] l10n_it_riba: It is already named l10n_it_riba_oca#5627

Merged
pedrobaeza merged 1 commit into
OCA:18.0from
PyTech-SRL:18.0-rem-riba
May 20, 2026
Merged

[REM] l10n_it_riba: It is already named l10n_it_riba_oca#5627
pedrobaeza merged 1 commit into
OCA:18.0from
PyTech-SRL:18.0-rem-riba

Conversation

@SirPyTech

Copy link
Copy Markdown

The rename has been merged a few days ago: OCA/l10n-italy#4856.

@sergiocorato

Copy link
Copy Markdown

Please respect commit name guide-line, like [18.0][OU-FIX] etc
Thanks

@SirPyTech

SirPyTech commented May 18, 2026

Copy link
Copy Markdown
Author

Thanks for having a look!

Please respect commit name guide-line, like [18.0][OU-FIX] etc
Thanks

There is no specific guideline for this project's commit name, see https://github.com/OCA/OpenUpgrade/blob/18.0/CONTRIBUTING.md#project-specific-guidelines:

This project does not have specific coding guidelines.

but looks like everyone is creating their own instead 🤔 image

So I followed the usual guidelines in https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst; I changed the commit message anyway because [IMP] seems more appropriate.

@sergiocorato

Copy link
Copy Markdown

Thanks for having a look!

Please respect commit name guide-line, like [18.0][OU-FIX] etc
Thanks

There is no specific guideline for this project's commit name, see https://github.com/OCA/OpenUpgrade/blob/18.0/CONTRIBUTING.md#project-specific-guidelines:

This project does not have specific coding guidelines.

but looks like everyone is creating their own instead 🤔 image

So I followed the usual guidelines in https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst; I changed the commit message anyway because [IMP] seems more appropriate.

This is probably a not written recent guideline, see #5098 (comment)

@remi-filament remi-filament left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for your PR @SirPyTech
You are right there is no guideline for commit as there are divergent view whether OU- should be added between brackets or not.
From my pov, this should be a FIX rather than a IMP and should not mention openupgrade_scripts but apriori instead.

Also it is useful for reviewers if you can add version number in PR name, [18.0]...

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza pedrobaeza added this to the 18.0 milestone May 20, 2026
@pedrobaeza

Copy link
Copy Markdown
Member

This rename in 16 then seems incorrect:

https://github.com/OCA/OpenUpgrade/blob/16.0/openupgrade_scripts/apriori.py#L66

Isn't it?

@SirPyTech

Copy link
Copy Markdown
Author

This rename in 16 then seems incorrect:

https://github.com/OCA/OpenUpgrade/blob/16.0/openupgrade_scripts/apriori.py#L66

Isn't it?

Thanks for having a look!

Let me summarize the situation first:

So the rename you linked for 16.0:

"l10n_it_ricevute_bancarie": "l10n_it_riba_oca",

is actually correct.

And remember that all this mess is because Odoo added an enterprise module named l10n_it_riba in 16.0 😇

@SirPyTech

Copy link
Copy Markdown
Author

From my pov, this should be a FIX rather than a IMP and should not mention openupgrade_scripts but apriori instead.

Thanks for having a look!
I understand, but in the commit title I just mentioned the module and why the change happened, the detailed implementation (editing apriori) is already shown in the commit's content; thanks for approving either way.

Also it is useful for reviewers if you can add version number in PR name, [18.0]...

I don't really like repeating the version information in multiple places, because we already have that same information in the base branch and now also in a dedicated label; I feel like adding it in the PR title is redundant.
I review PRs too and feel like the version in the PR title is not needed.

@pedrobaeza

Copy link
Copy Markdown
Member

OK, let's continue and let the commit message aside.

@pedrobaeza pedrobaeza merged commit d09833f into OCA:18.0 May 20, 2026
4 checks passed
@SirPyTech

Copy link
Copy Markdown
Author

OK, let's continue and let the commit message aside.

Thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants