[16.0][MIG] auth_oauth_multi_token: Migration to 16.0

[ChrisOForgeFlow]

Reopen from #513

@ForgeFlow

[ramiadavid]

@ChrisOForgeFlow
Please, can you make the changes that @pedrobaeza requires in this comment?
#513 (comment)

[mlaitinen]

@ChrisOForgeFlow Please, can you make the changes that @pedrobaeza requires in this comment? #513 (comment)

There's a surprising eagerness to enforce a rule that doesn't seem exist in the OCA guidelines.

[ramiadavid]

It's simply being practical, if whoever can do the merge tells me to change it, I change it so as not to delay it.

On the other hand, from a personal point of view, I consider that there is no reason in a migration to make changes that in no way affect its operation, simply because I like it more one way or another, but rather to do the minimum. possible changes, so that future fixes are easier to apply across different versions.

And getting into unnecessary discussions, the only thing that usually results in is delaying the availability of the module, meaning that practically after 1 year this module is still in process. I prefer to migrate the module as it is and in any case make another pull request afterwards to apply those changes, in which case if it is delayed we can use the module normally.

[pedrobaeza]

@mlaitinen @ramiadavid the principles to ask for my change are:

• Reduce the diff at maximum. This way, any query to debug a problem is not distorted with changes that don't impact in the migration. We are all impulsed to do some "refactoring" sometimes to match a style or preference at the same time we do other task, like migrations, I know that, but the way to do it if we want this is to isolate these changes in a specific commit. And in the end, it doesn't worth the change most of the times. I have touched modules that are not mine, and ending up simply going to the specific patch.

• I haven't found a clear reference from an official institution about the capitalization you mention. First of all, it's dubious what a title refers to. Some of the places (unofficial) mention titles of songs, books, etc, but these are not titles, but other texts. Recalling the search today, I have the same doubts and differences of criteria looking at these places:

  https://www.scribbr.com/language-rules/capitalization-rules/
  https://www.grammarly.com/blog/capitalization-rules/
  https://editorsmanual.com/articles/capitalization/
  https://www.grammarbook.com/punctuation/capital.asp

• Finally, and the most important one, changing the texts, you are leading to lose all the existing translations, which are very valuable, and shouldn't be discarded by no strong reason. You can see the impact already looking at the POT diff: 2c90556#diff-a0d653fd478f88e629cc7b74bc2a2f401ac60e6380be48d96044a180847ac872. This can be minimized replacing the msgid on each of the .po, but it's not the case.

Do you still see my comment as irrelevant?

[mlaitinen]

Reduce the diff at maximum. This way, any query to debug a problem is not distorted with changes that don't impact in the migration. We are all impulsed to do some "refactoring" sometimes to match a style or preference at the same time we do other task, like migrations, I know that, but the way to do it if we want this is to isolate these changes in a specific commit. And in the end, it doesn't worth the change most of the times. I have touched modules that are not mine, and ending up simply going to the specific patch.

Having reviewed plenty of PRs, I generally agree on minimizing the diff, but mostly when the changes don't have any effect at all (e.g., unnecessary code style changes), or when the commit gets bloated to the point where it actually makes the review unnecessarily difficult. My commit had 11 changes directly attributable to the corrections in capitalization, which I don't think is too much to make the review difficult, and I also don't think the changes were unnecessary.

You have a good point about isolating these kind of changes to a separate commit, but I wasn't sure if migration PRs with more than one [MIG] commit would've been accepted, let alone a separate commit before or after merging the PR, since it's unlikely that anyone would use their precious time to review a PR with format changes only.

I haven't found a clear reference from an official institution about the capitalization you mention.

I realize now I should have explained my change as "making the capitalization consistent with Odoo's other titles", rather than following English capitalization rules. If all other field names and captions in Odoo wouldn't follow the capitalization rules, of course I would adapt to that, to maintain the consistency of the user interface. In the screenshot below, I marked all of the captions of this module in red, and all Odoo captions with blue:

Odoo, by default, capitalizes the words in the titles in cases where the explicit field string is omitted.

Finally, and the most important one, changing the texts, you are leading to lose all the existing translations, which are very valuable, and shouldn't be discarded by no strong reason.

While it's a valid argument in general, it would only apply to this particular case if any translations existed when I submitted the PR. There were none in May 2023.

Do you still see my comment as irrelevant?

Absolutely not, and never did. We can all see your valuable work that you do for OCA and Odoo. Thank you for that, and for addressing this question.

[pedrobaeza]

OK, thanks for the extra explanations, Miku. I don't think we should follow all the Odoo decisions (look at the OCA guidelines that are better than the Odoo guidelines), and this subject (capitals in all places) personally annoys me (the same as accountant people writing all in uppercase), but it's not a big deal for this case, and as you have said, there are no translations of the module for now, so let's continue and unblock the thing.

/ocabot migration auth_oauth_multi_token
/ocabot merge nobump

but I wasn't sure if migration PRs with more than one [MIG] commit would've been accepted, let alone a separate commit before or after merging the PR, since it's unlikely that anyone would use their precious time to review a PR with format changes only.

The commit should be [IMP] for this case (or [REF] when there are code refactors).

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-584-by-pedrobaeza-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at efac732. Thanks a lot for contributing to OCA. ❤️