On Mon, May 25, 2020 at 8:34 PM Burakov, Anatoly
<anatoly.bura...@intel.com> wrote:
>
> On 25-May-20 1:08 PM, Bruce Richardson wrote:
> > On Mon, May 25, 2020 at 12:12:49PM +0100, Burakov, Anatoly wrote:
> >> On 25-May-20 10:34 AM, Morten Brørup wrote:
> >>> Dear DPDK Techboard,
> >>>
> >>> I am writing this to raise awareness about the environment for
> >>> contributing to DPDK, as I feel that it could be improved. This is not a
> >>> personal thing - I have thick skin - but a general observation. I urge
> >>> the DPDK Techboard to spend some time to focus on the process, and not
> >>> only on the technology.
> >>>
> >>> Contributing to DPDK is not easy for infrequent contributors:
> >>>
> >>> 1. Infrequent contributors are limited by not being deeply familiar with
> >>> the coding style and the commit style, so their style is not always 100 %
> >>> spot on.
> >>> 2. Infrequent contributors are limited by not having built trust by the
> >>> maintainers and frequent contributors, and thus their contributions
> >>> undergo more detailed reviews and get more negative (or: perceived
> >>> negative) feedback, where trusted contributors are given more slack. (In
> >>> theory, every contribution should be treated equal, but in reality it
> >>> makes sense allocating fewer resources to review contributions from
> >>> developers with a proven track record.)
> >>> 3. Infrequent contributors may not be deeply familiar with the
> >>> development/contribution tools. E.g. how to use git the "DPDK way".
> >>>
> >>> Additionally, when contributing to old DPDK code, checkpatch complains
> >>> about coding style violations stemming from the existing old code. This
> >>> also raises the barrier and decreases the motivation to contribute - a
> >>> contributor getting negative feedback about something he didn't even do.
> >>>
> >>>
> >>> Here are a couple of anonymous examples from the mailing list:
> >>>
> >>> An infrequent contributor got minor coding style suggestions to a patch,
> >>> although the coding style was similar to that of a closely related
> >>> function in the same library, but not perfectly matching the official
> >>> coding style. I think we could be more lax about coding style, except if
> >>> the coding style directly violates automatic coding style validation
> >>> tools.
> >>>
> >>
> >> A lot of that could simply be fixed by codifying our Coding Style into a
> >> .clang-format file, and make this process (semi-)automatic. A lot of
> >> IDE's/editors now have either built-in support for clang-format, or have
> >> plugins enabling said support.
> >>
> >> I've investigated this in the past and found that our coding style
> >> guidelines are very specific in some places, and neither clang-format nor
> >> other options have that kind of detailed control over source code
> >> formatting. The only other option would be to adjust our coding style to
> >> fit
> >> the options available in clang-format.
> >>
> >> IMO this would cut down a lot on complaints about mixing indents, wrong
> >> alignment, (lack of) newlines before function name, etc.
> >>
> >
> > This is of definite interest to me, for one. How close to our current
> > standards can we get right now with clang-format? If the coding standards
> > right now can't match exactly, how big would be the changes to make them
> > doable in clang-format? Is it one or two things, or is it quite a number?
> >
> > /Bruce
> >
>
> The issues weren't that serious - mainly the peculiarities around our
> custom macros like __rte_experimental or our specific flavor of
> indentation. I can't remember off the top of my head exactly what was
> the problem as i've looked at this a couple of years ago, but i'm pretty
> sure the majority of the stuff we expect in DPDK is configurable through
> clang-format. Plus, i'm sure clang-format has gained features in the
> meantime, maybe the things that weren't possible then, are now :)
I have been using the following clang-format configuration for DPDK[1].
I dont see any setbacks. ForEachMacros section below needs to extend for DPDK.
[1]
"{ BasedOnStyle: LLVM, IndentWidth: 8, TabWidth: 8, UseTab: Always,
AllowShortIfStatementsOnASingleLine: false, IndentCaseLabels: false,
ColumnLimit: 80, AllowShortFunctionsOnASingleLine: false,
AlwaysBreakAfterReturnType: AllDefinitions, ColumnLimit: 80,
ConstructorInitializerAllOnOneLineOrOnePerLine: true,
ConstructorInitializerIndentWidth: 8, ContinuationIndentWidth: 8,
BreakBeforeBraces: Linux, AllowShortBlocksOnASingleLine: false,
AlignConsecutiveAssignments: false, AlignEscapedNewlines: Right,
AlignConsecutiveMacros : true, MaxEmptyLinesToKeep : 1,
Cpp11BracedListStyle : true, AlignTrailingComments : true,
ForEachMacros: ['STAILQ_FOREACH', 'rte_graph_foreach_node',
'TAILQ_FOREACH', 'RTE_ETH_FOREACH_DEV']}",
>
> --
> Thanks,
> Anatoly
