docs(CONTRIBUTING): Proposed updates to Contributing Guidelines

[rlalik]

Following up discussion from #1628 and proposition by @ChristianTackeGSI to update Contributing Guidelines with my ideas, I open this PR in Draft mode to work on updates.

N: Things I did now:

1. Separated git-related rules into separate V (Version Control) category
2. Added new G.3 rule category about Other C++ Guidelines, for those not matching G.2 related to CPPCG
3. Added G.3.1 as follow up to FairModule IsSensitive checks medium property not volume name #1628 about use of ROOT/standard types
4. Updated Contributing Code.2 rule and added other files to the list which need to be updated by the new contributors.

S: Things I would (strongly) suggest to update:

1. Enumerate all guidelines instead of using bullets - it helps to refer to them by number, e.g. G.3.1.2
2. Specify exact clang-format version - different version may produce different result for the same config file. This update should be followed by update of apply-format.sh and check-format.sh (as separate PR).

O: Other ideas

1. Could the rule:

• In rare cases (e.g. backports, some hotfixes) base against the appropriate branch

be removed and commit cherry-picking be used instead (after being agreed in the PR comment section)? New rule could be:

• If change should be applied to other branches, cherry picking is the preferred way to go.

Please refer to these list as N.1, S.2, O.1, etc... in the discussion section.

---

Checklist:

• Followed the Contributing Guidelines

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f16c2627-ad9e-4d5b-b019-d06b7a76c93e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dennisklein]

CONTRIBUTORS is the authoritative place to add new names. Maintainers will then run meta_update.py (ref N.4)

[rlalik]

Oh, ok, from this comment I got impression that I should do it by myself: #1628 (comment)
I'll restore older version to this part.

[dennisklein]

What is your rationale for N.1 (breaking all existing references to the renamed rules) and N.2 (reusing an already used rule identifier for different semantics)?

[dennisklein]

O.1: Every push shall happen via a pull request. The current rules do not disallow cherry-picks, in fact, that is what usually is done in PR branches of backports and hotfixes.

[dennisklein]

S.1 👍

[dennisklein]

S.2 That's an annoying one indeed. It is unfortunate, clang-format is breaking formatting so often. I am just not sure, specifying some version here, will improve anything - we would have to bump it eventually, what will be the user experience then? We have briefly discussed internally whether we should target always the latest clang-format release or if we should point the user to the version used in the CI (which is currently down), but without a conclusion so far.

[rlalik]

What is your rationale for N.1 (breaking all existing references to the renamed rules) and N.2 (reusing an already used rule identifier for different semantics)?

I'm not sure how often the rules were quoted in the past. Is this serious issues? Rationale is:

1. For old commits the reference will refer to guidelines from that time. So shouldn't be a bug issue - is anyone going to check old commit messages for references?

2. There are sperate categories for style etc, yet git usage and coding conventions are put under the same category. Separating them would for into pattern.

3. If the new G.3 was put into older G Category after git rules, that would be a bit mess reading coding rules one would expect them having next to each other, than being separated by git rules.

4. Perhaps these changes are breaking, but also rules are short for now. It's easier to break and reorganise now than when they expand. Then it would be a bigger mess. Perhaps it's now time to rethink the categories, sort, group, and then keep the pattern for future.

I believe there is some point in this reason.

[rlalik]

O.1: Every push shall happen via a pull request. The current rules do not disallow cherry-picks, in fact, that is what usually is done in PR branches of backports and hotfixes.

Perhaps this proposal front my side is indeed invalid. So, the cherry picking is done from other branch which was pull requested into dev, right? So it's other direction than I suggested. It's fine, it was just suggestion and I won't die on this hill.

[rlalik]

S.2 That's an annoying one indeed. It is unfortunate, clang-format is breaking formatting so often. I am just not sure, specifying some version here, will improve anything - we would have to bump it eventually, what will be the user experience then? We have briefly discussed internally whether we should target always the latest clang-format release or if we should point the user to the version used in the CI (which is currently down), but without a conclusion so far.

Yes, this is tricky. Different distros will have different latest version. Should we use Ubuntu LTS as a reference? Then we are always 2-3 releases behind. Which is fine I think.

One does not need to reformat whole code. It's enough to run git clang-format --staged and then only new code will be formatted. If there was clang-format version bump, it will not reformat the whole codebase.

Also, I think cbmroot has it done nicely in their CI.