OCA
[16.0][IMP] dms_field: Propagate groups from field template
With this PR, the groups declared in child directories of the field template are propagated to the directories created in the target record.
I have also included a couple of small refactorings, in their own commit.
Let me know what you think!
I think all these changes can be combined into a single commit: [IMP] dms_field: Propagate groups from field template
The title of the PR should be changed to something like: [16.0][IMP] dms_field: Propagate groups from field template
I think all these changes can be combined into a single commit: [IMP] dms_field: Propagate groups from field template
I prefer to highlight which commits do not change the behavior ([REF] ones) from the other ones, and to keep each commit focused on their change, so I'd like to keep them all.
The title of the PR should be changed to something like: [16.0][IMP] dms_field: Propagate groups from field template
Right, added more details in the PR title, I'm not a fan of including the version in the PR title though.
If you need that for filters, note that PRs can be filtered by version 16.0 using base:16.0 filter (see https://github.com/OCA/dms/pulls?q=is%3Apr+is%3Aopen+base%3A16.0).
@SirPyTech the version has to be in the PR title, not in the commit message.
The title of the PR should be changed to something like: [16.0][IMP] dms_field: Propagate groups from field template
Right, added more details in the PR title, I'm not a fan of including the version in the PR title though. If you need that for filters, note that PRs can be filtered by version
16.0usingbase:16.0filter (see https://github.com/OCA/dms/pulls?q=is%3Apr+is%3Aopen+base%3A16.0).
@SirPyTech the version has to be in the PR title, not in the commit message.
Why does it "have to be" there?
If that's used for filtering the PRs, the dedicated filter base already exists as I wrote above
Current convention is for seeing in a glance which is the target branch from the pull request list or the received notification mails, without having to navigate to the PR itself to see it. Please make reviewers' life easier.
Current convention is for seeing in a glance which is the target branch from the pull request list or the received notification mails, without having to navigate to the PR itself to see it. Please make reviewers' life easier.
I am a reviewer too and I have never needed the version in the title because I'm using the base filter, and I think that should be the way for searching PRs of a specific version; in the rare occasions I needed more, I used GitHub's API.
Could you please propose this convention to the Guidelines in https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst? The contributors already have so many guidelines to follow, it would be nice if they were at least all written somewhere.
When this new convention is in the guidelines, maybe the OCA bot could help enforcing it :robot: and you wouldn't need to ask all the contributors to edit their PR's title.
There's no section in guidelines for that, and I'm not going to lose more time in that bureaucracy given the result in other attempts, but if someone asks you for that, and that would ease the review, please do it as I do. If not, then we will be reluctant to review your PRs. It's that simple.
My comment is still pending https://github.com/OCA/dms/pull/441#issuecomment-3196129887
I think all these changes can be combined into a single commit: [IMP] dms_field: Propagate groups from field template
IMO, all changes can be combined in a single commit; having several in this case adds nothing.
My comment is still pending #441 (comment)
I think all these changes can be combined into a single commit: [IMP] dms_field: Propagate groups from field template
IMO, all changes can be combined in a single commit; having several in this case adds nothing.
Why is this case special? By the way, I already answered that one in https://github.com/OCA/dms/pull/441#issuecomment-3196190899:
I prefer to highlight which commits do not change the behavior (
[REF]ones) from the other ones, and to keep each commit focused on their change, so I'd like to keep them all.
It happens to have different opinions :shrug: I think having different commits makes more sense and is more aligned with the guidelines:
Only make a single commit per logical change set.
(from https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message)
Rewriting a method to allow override is a logical change set, and that deserves a commit on its own. Changing the behavior of a method is another logical change set too, and that deserves a commit on its own.
Would you agree with a compromise solution of having one [REF] commits that includes the existing [REF] commits, and one [IMP] commit?
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). 🤖
