docs: Add styleguide.
Adds styleguide document. I added links to the ALICE styleguide and outlined the changes / exceptions we agreed on.
Additionally some adjustments to clang-format:
-
AccessModifierOffset: -1
: Indent access specifier by 1 space. For consistency with ALICE. Not sure this change is necessary. ALICE also formats class brackets differently. And on it's own I don't see any benefit over the current format. -
AllowShortBlocksOnASingleLine: Never
+AllowShortIfStatementsOnASingleLine: Never
Enforces line break afterif
. So allowed formattings are now:
if (someTest)
foo();
and
if (someTest) {
foo();
}
Again this is now identical to ALICE. Changes we discussed to always add the brackets isn't possible with our version of clang-format.
- Not discussed during the meeting:
- Regex: '".*"'
Priority: 1
This should only affect online code. Puts includes from within cbmroot (included via ""
) before external includes (that should be included via <>
). This is necessary because online code drops the Cbm
prefix from file names that is used to distinguish cbmroot headers.
-
SpaceBeforeCpp11BracedList: false
: ALICE styleguide recommends using brace initialization.
std::vector<std::string> myVector {"alpha", "beta", "gamma"};
// becomes
std::vector<std::string> myVector{"alpha", "beta", "gamma"};
This unifies initialization for c structs and c++ classes.
Requesting feedback from: @p.-a.loizeau, @d.smith, @j.decuveland, @s.zharko, @se.gorbunov, @f.uhlig, @v.friese
Merge request reports
Activity
requested review from @v.friese
assigned to @v.friese
added 8 commits
-
fe2eb125...fa6e31d8 - 6 commits from branch
computing:master
- 1f40539e - docs: Add styleguide.
- cc734512 - Update clang-format.
-
fe2eb125...fa6e31d8 - 6 commits from branch
Dear @f.uhlig, @v.friese, @p.-a.loizeau,
you have been identified as code owner of at least one file which was changed with this merge request.
Please check the changes and approve them or request changes.
added CodeOwners label
- Resolved by Volker Friese
Changes we discussed to always add the brackets isn't possible with our version of clang-format.
which version of clang-format would we need?
Since we are on the topic: Can we do something about the unrolling of initializer lists?
I.e., in the following example:
Hitfind::Hitfind(tof::HitfindSetup setup, bool QaMode) : fNbSm(setup.NbSm) , fNbRpc(setup.NbRpc) , fStorDigi(fNbSm.size()) { ... }
This is what clang-format currently generates. Does this need to be one line for each?
I would propose the following:
Hitfind::Hitfind(tof::HitfindSetup setup, bool QaMode) : fNbSm(setup.NbSm) , fNbRpc(setup.NbRpc) , fStorDigi(fNbSm.size()) { ... }
If the parameter list gets longer, I would then simply indent by one more level, i.e.
Hitfind::Hitfind(tof::HitfindSetup setup, bool QaMode) : fNbSm(setup.NbSm) , fNbRpc(setup.NbRpc) , fStorDigi(fNbSm.size()) , more parameters ... , more parameters ... { ... }
- Resolved by Felix Weiglhofer
added 33 commits
-
cc734512...f025ce83 - 29 commits from branch
computing:master
- 5e67cfbb - docs: Add styleguide.
- 5a89d701 - Update clang-format.
- 8b1c1912 - Fix include order.
- d2567e1f - Allow short ifs on a single line (without else branch).
Toggle commit list-
cc734512...f025ce83 - 29 commits from branch
@v.friese: Can we merge? Immediate concerns about
if
formatting should be addressed with the change toWithoutElse
option and we can revisit once we have the new clang-format. And we will never address all formatting issues in a single MR.Edited by Felix Weiglhoferenabled an automatic merge when the pipeline for d2567e1f succeeds
mentioned in merge request !1525 (merged)
added Documentation label