[dpdk-dev] Consider improving the DPDK contribution processes

[dpdk-dev] Consider improving the DPDK contribution processes

[Morten Brørup]

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.

Another infrequent contributor got patch style feedback about a small patch, suggesting to split it up into three patches. The official contributing guide says that small changes should be kept together in the same patch. The patch in question could be considered three bug fixes, so splitting it up might be appropriate, or it could be considered fixing three variations of the same bug, so keeping it together as one would be appropriate. I think we could be more lax about patch style, except if the patches are completely incomprehensible.

It is not all bad, though! I have more than once seen excellent feedback to a suggested patch, where an expert reviewer took the time and effort to explain - in an educational and welcoming tone - what was wrong about the patch and the contributor's assumptions.


In summary, I think that DPDK has grown to a point where we hopefully will see more contributions from outsiders and newcomers, and we need to adapt to it, so that they get a positive experience from contributing, and keep coming back with more.

Improving the process should be an ongoing discussion. Here are two areas for discussion:

1. Keep expanding the contributor documentation. It is already far better than for most projects, but there is always room for improvement. Assume that the reader is intelligent, but has limited information in advance. E.g. Patchwork and Bugzilla are barely mentioned, and it is only described how to use them when submitting a patch.

2. When giving feedback on the mailing list, consider how it may be received, weighing the balance between accepting imperfection and educating the contributor.


DPDK is a great project, and the contribution process described above sounds a lot worse than it actually is. But I just can't get enough rainbows and unicorns! :-)


Med venlig hilsen / kind regards

Morten Brørup
CTO


SmartShare Systems A/S
Tonsbakken 16-18
DK-2740 Skovlunde
Denmark

Office      +45 70 20 00 93
Direct      +45 89 93 50 22
Mobile     +45 25 40 82 12

mb@smartsharesystems.com
www.smartsharesystems.com

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Jerin Jacob]

>
> 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.
>
> Another infrequent contributor got patch style feedback about a small patch, suggesting to split it up into three patches. The official contributing guide says that small changes should be kept together in the same patch. The patch in question could be considered three bug fixes, so splitting it up might be appropriate, or it could be considered fixing three variations of the same bug, so keeping it together as one would be appropriate. I think we could be more lax about patch style, except if the patches are completely incomprehensible.
>

A lot of patch style can be fixed automatically through clang-format.

Please find below the Linux kernel documentation for its clang-format and usage.

https://www.kernel.org/doc/html/latest/process/clang-format.html
https://github.com/torvalds/linux/blob/master/.clang-format

Maybe we could add .clang-format file for DPDK.
I have been using the following clang-format configuration for DPDK[1]
May be for once, we need to fix the coding standard issue with the
existing codebase using clang-format.

[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']}",

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

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.

> Another infrequent contributor got patch style feedback about a small patch, suggesting to split it up into three patches. The official contributing guide says that small changes should be kept together in the same patch. The patch in question could be considered three bug fixes, so splitting it up might be appropriate, or it could be considered fixing three variations of the same bug, so keeping it together as one would be appropriate. I think we could be more lax about patch style, except if the patches are completely incomprehensible.

In my experience, this has been up to individual maintainers. I 
generally split up my patches not to follow a guideline, but to make 
reviewing code easier for others. Sometimes that means splitting up the 
patch, sometimes that means keeping everything together, and sometimes 
either one is OK so i don't insist on either.

But perhaps it would be a good idea to clarify some of these guidelines.

Then again, we should also remember that coding guidelines are there to 
help us all, and to avoid wasting maintainers' time: David/Thomas/Ferruh 
manually fixing every patch before applying just because we are "lax" in 
reviews doesn't really scale. So, it's really a double edged sword IMO.

> 
> It is not all bad, though! I have more than once seen excellent feedback to a suggested patch, where an expert reviewer took the time and effort to explain - in an educational and welcoming tone - what was wrong about the patch and the contributor's assumptions.
> 
> 
> In summary, I think that DPDK has grown to a point where we hopefully will see more contributions from outsiders and newcomers, and we need to adapt to it, so that they get a positive experience from contributing, and keep coming back with more.
> 
> Improving the process should be an ongoing discussion. Here are two areas for discussion:
> 
> 1. Keep expanding the contributor documentation. It is already far better than for most projects, but there is always room for improvement. Assume that the reader is intelligent, but has limited information in advance. E.g. Patchwork and Bugzilla are barely mentioned, and it is only described how to use them when submitting a patch.
> 
> 2. When giving feedback on the mailing list, consider how it may be received, weighing the balance between accepting imperfection and educating the contributor.

I would add a third area: the process itself is arcane and inaccessible. 
The current consensus among the community seems to be that IRC + mailing 
list are "the most accessible" because "everyone has email" and "getting 
on IRC is easy".

However, the truth is, they aren't "accessible", they are low tech, and 
thus *in*accessible to those who aren't veteran command-line Linux 
coders. No one uses IRC any more except OSS projects (so a new 
contributor isn't likely to have an IRC set up unless they're already a 
habitual contributor to other projects), and sending patches over an 
email as opposed to a well-integrated web interface workflow is so alien 
to most people that it definitely does discourage new contributions.

I understand the advantages of mailing lists (vendor independence, 
universal compatibility, etc.), but after doing reviews in Github/Gitlab 
for a while (we use those internally), going through DPDK mailing list 
and reviewing code over email fills me with existential dread, as the 
process feels so manual and 19th century to me.

> 
> 
> DPDK is a great project, and the contribution process described above sounds a lot worse than it actually is. But I just can't get enough rainbows and unicorns! :-)
> 
> 
> Med venlig hilsen / kind regards
> 
> Morten Brørup
> CTO
> 
> 
> SmartShare Systems A/S
> Tonsbakken 16-18
> DK-2740 Skovlunde
> Denmark
> 
> Office      +45 70 20 00 93
> Direct      +45 89 93 50 22
> Mobile     +45 25 40 82 12
> 
> mb@smartsharesystems.com
> www.smartsharesystems.com
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Jerin Jacob]

> I would add a third area: the process itself is arcane and inaccessible.
> The current consensus among the community seems to be that IRC + mailing
> list are "the most accessible" because "everyone has email" and "getting
> on IRC is easy".
>
> However, the truth is, they aren't "accessible", they are low tech, and
> thus *in*accessible to those who aren't veteran command-line Linux
> coders. No one uses IRC any more except OSS projects (so a new

I agree. Since IRC is not secure, a lot of companies are blocking the 6667 port.
Another alternative is to see _slack_ "free public" channels. Some of
the cool features of
new tools are really useful. like notification, search in chat
area(help to create the knowledge base),
mobile application support, tons of custom free apps for a specific
workflow, etc.

> contributor isn't likely to have an IRC set up unless they're already a
> habitual contributor to other projects), and sending patches over an
> email as opposed to a well-integrated web interface workflow is so alien
> to most people that it definitely does discourage new contributions.
>
> I understand the advantages of mailing lists (vendor independence,
> universal compatibility, etc.), but after doing reviews in Github/Gitlab
> for a while (we use those internally), going through DPDK mailing list
> and reviewing code over email fills me with existential dread, as the
> process feels so manual and 19th century to me.

Agree. I had a difference in opinion when I was not using those tools.
My perspective changed after using Github and Gerrit etc.

Github pull request and integrated public CI(Travis, Shippable ,
codecov) makes collaboration easy.
Currently, in patchwork, we can not assign a patch other than the set
of maintainers.
I think, it would help the review process if the more fine-grained
owner will be responsible for specific
patch set.

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Thomas Monjalon]

25/05/2020 13:58, Jerin Jacob:
> 25/05/2020 11:34, Morten Brørup:
> > sending patches over an
> > email as opposed to a well-integrated web interface workflow is so alien
> > to most people that it definitely does discourage new contributions.
> >
> > I understand the advantages of mailing lists (vendor independence,
> > universal compatibility, etc.), but after doing reviews in Github/Gitlab
> > for a while (we use those internally), going through DPDK mailing list
> > and reviewing code over email fills me with existential dread, as the
> > process feels so manual and 19th century to me.
> 
> Agree. I had a difference in opinion when I was not using those tools.
> My perspective changed after using Github and Gerrit etc.
> 
> Github pull request and integrated public CI(Travis, Shippable ,
> codecov) makes collaboration easy.
> Currently, in patchwork, we can not assign a patch other than the set
> of maintainers.
> I think, it would help the review process if the more fine-grained
> owner will be responsible for specific
> patch set.

The more fine-grain is achieved with Cc in mail.
But I understand not everybody knows/wants/can configure correctly
an email client. Emails are not easy for everybody, I agree.

I use GitHub as well, and I really prefer the clarity of the mail threads.
GitHub reviews tend to be line-focused, messy and not discussion-friendly.
I think contribution quality would be worst if using GitHub.

There is a mailing list discussing workflow tooling:
	https://lore.kernel.org/workflows/

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

On 25-May-20 1:53 PM, Thomas Monjalon wrote:
> 25/05/2020 13:58, Jerin Jacob:
>> 25/05/2020 11:34, Morten Brørup:
>>> sending patches over an
>>> email as opposed to a well-integrated web interface workflow is so alien
>>> to most people that it definitely does discourage new contributions.
>>>
>>> I understand the advantages of mailing lists (vendor independence,
>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>> for a while (we use those internally), going through DPDK mailing list
>>> and reviewing code over email fills me with existential dread, as the
>>> process feels so manual and 19th century to me.
>>
>> Agree. I had a difference in opinion when I was not using those tools.
>> My perspective changed after using Github and Gerrit etc.
>>
>> Github pull request and integrated public CI(Travis, Shippable ,
>> codecov) makes collaboration easy.
>> Currently, in patchwork, we can not assign a patch other than the set
>> of maintainers.
>> I think, it would help the review process if the more fine-grained
>> owner will be responsible for specific
>> patch set.
> 
> The more fine-grain is achieved with Cc in mail.
> But I understand not everybody knows/wants/can configure correctly
> an email client. Emails are not easy for everybody, I agree.
> 
> I use GitHub as well, and I really prefer the clarity of the mail threads.
> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
> I think contribution quality would be worst if using GitHub.

I have more experience with Gitlab than Github, but i really don't see 
it that way.

For one, reviewing in Gitlab makes it easier to see context in which 
changes appear. I mean, obviously, you can download the patch, apply it, 
and then do whatever you want with it in your editor/IDE, but it's just 
so much faster to do it right in the browser. Reviewing things with 
proper syntax highlighting and side-by-side diff with an option to see 
more context really makes a huge difference and is that much faster.

I would also vehemently disagree with the "clarity" argument. There is 
enforced minimum standard of clarity of discussion in a tool such as 
Gitlab. I'm sure you noticed that some people top-post, some 
bottom-post. Some will remove extraneous lines of patches while some 
will leave on comment in a 10K line patch and leave the rest as is, in 
quotes. Some people do weird quoting where they don't actually quote but 
just copy text verbatim, making it hard to determine where the quote 
starts. If the thread is long enough, you'd see the same text quoted 
over and over and over. All of that is not a problem within a single 
patch email, but it adds up to lots of wasted time on all sides.

And all of the above will not be a problem with a tool like 
Gitlab/Github. There are "general" comments that can be used for general 
discussion, and there are line-specific comments that can be used to 
discuss certain sections of the patch. I've done this many times in many 
reviews, and it works very well. Now, granted, I've never maintained an 
entire repository like DPDK, so you may have a different perspective, 
but i really don't see how long email chains have "clarity" that a 
discussion thread with proper quoting, links to code, markdown syntax, 
etc. doesn't.

(for the record, i don't consider Gerrit to be a good tool because it 
enforces a particular git workflow, one that is not at all compatible 
with how our community works. GitLab, on the other hand, "just works" - 
i'm assuming GitHub is very similar)

> 
> There is a mailing list discussing workflow tooling:
> 	https://lore.kernel.org/workflows/
> 
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 25, 2020, at 9:28 AM, Burakov, Anatoly <anatoly.burakov@intel.com> wrote:
> 
> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>> 25/05/2020 13:58, Jerin Jacob:
>>> 25/05/2020 11:34, Morten Brørup:
>>>> sending patches over an
>>>> email as opposed to a well-integrated web interface workflow is so alien
>>>> to most people that it definitely does discourage new contributions.
>>>> 
>>>> I understand the advantages of mailing lists (vendor independence,
>>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>>> for a while (we use those internally), going through DPDK mailing list
>>>> and reviewing code over email fills me with existential dread, as the
>>>> process feels so manual and 19th century to me.
>>> 
>>> Agree. I had a difference in opinion when I was not using those tools.
>>> My perspective changed after using Github and Gerrit etc.
>>> 
>>> Github pull request and integrated public CI(Travis, Shippable ,
>>> codecov) makes collaboration easy.
>>> Currently, in patchwork, we can not assign a patch other than the set
>>> of maintainers.
>>> I think, it would help the review process if the more fine-grained
>>> owner will be responsible for specific
>>> patch set.
>> The more fine-grain is achieved with Cc in mail.
>> But I understand not everybody knows/wants/can configure correctly
>> an email client. Emails are not easy for everybody, I agree.
>> I use GitHub as well, and I really prefer the clarity of the mail threads.
>> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
>> I think contribution quality would be worst if using GitHub.
> 
> I have more experience with Gitlab than Github, but i really don't see it that way.
> 
> For one, reviewing in Gitlab makes it easier to see context in which changes appear. I mean, obviously, you can download the patch, apply it, and then do whatever you want with it in your editor/IDE, but it's just so much faster to do it right in the browser. Reviewing things with proper syntax highlighting and side-by-side diff with an option to see more context really makes a huge difference and is that much faster.
> 
> I would also vehemently disagree with the "clarity" argument. There is enforced minimum standard of clarity of discussion in a tool such as Gitlab. I'm sure you noticed that some people top-post, some bottom-post. Some will remove extraneous lines of patches while some will leave on comment in a 10K line patch and leave the rest as is, in quotes. Some people do weird quoting where they don't actually quote but just copy text verbatim, making it hard to determine where the quote starts. If the thread is long enough, you'd see the same text quoted over and over and over. All of that is not a problem within a single patch email, but it adds up to lots of wasted time on all sides.
> 
> And all of the above will not be a problem with a tool like Gitlab/Github. There are "general" comments that can be used for general discussion, and there are line-specific comments that can be used to discuss certain sections of the patch. I've done this many times in many reviews, and it works very well. Now, granted, I've never maintained an entire repository like DPDK, so you may have a different perspective, but i really don't see how long email chains have "clarity" that a discussion thread with proper quoting, links to code, markdown syntax, etc. doesn't.
> 
> (for the record, i don't consider Gerrit to be a good tool because it enforces a particular git workflow, one that is not at all compatible with how our community works. GitLab, on the other hand, "just works" - i'm assuming GitHub is very similar)
> 
>> There is a mailing list discussing workflow tooling:
>> 	https://lore.kernel.org/workflows/
> 

Bummer, I did not have to write my email, so +1000 to this one.
> 
> -- 
> Thanks,
> Anatoly

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Thomas Monjalon]

25/05/2020 16:28, Burakov, Anatoly:
> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
> > 25/05/2020 13:58, Jerin Jacob:
> >> 25/05/2020 11:34, Morten Brørup:
> >>> sending patches over an
> >>> email as opposed to a well-integrated web interface workflow is so alien
> >>> to most people that it definitely does discourage new contributions.
> >>>
> >>> I understand the advantages of mailing lists (vendor independence,
> >>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
> >>> for a while (we use those internally), going through DPDK mailing list
> >>> and reviewing code over email fills me with existential dread, as the
> >>> process feels so manual and 19th century to me.
> >>
> >> Agree. I had a difference in opinion when I was not using those tools.
> >> My perspective changed after using Github and Gerrit etc.
> >>
> >> Github pull request and integrated public CI(Travis, Shippable ,
> >> codecov) makes collaboration easy.
> >> Currently, in patchwork, we can not assign a patch other than the set
> >> of maintainers.
> >> I think, it would help the review process if the more fine-grained
> >> owner will be responsible for specific
> >> patch set.
> > 
> > The more fine-grain is achieved with Cc in mail.
> > But I understand not everybody knows/wants/can configure correctly
> > an email client. Emails are not easy for everybody, I agree.
> > 
> > I use GitHub as well, and I really prefer the clarity of the mail threads.
> > GitHub reviews tend to be line-focused, messy and not discussion-friendly.
> > I think contribution quality would be worst if using GitHub.
> 
> I have more experience with Gitlab than Github, but i really don't see 
> it that way.
> 
> For one, reviewing in Gitlab makes it easier to see context in which 
> changes appear. I mean, obviously, you can download the patch, apply it, 
> and then do whatever you want with it in your editor/IDE, but it's just 
> so much faster to do it right in the browser. Reviewing things with 
> proper syntax highlighting and side-by-side diff with an option to see 
> more context really makes a huge difference and is that much faster.

OK


> I would also vehemently disagree with the "clarity" argument. There is 
> enforced minimum standard of clarity of discussion in a tool such as 
> Gitlab. I'm sure you noticed that some people top-post, some 
> bottom-post. Some will remove extraneous lines of patches while some 
> will leave on comment in a 10K line patch and leave the rest as is, in 
> quotes. Some people do weird quoting where they don't actually quote but 
> just copy text verbatim, making it hard to determine where the quote 
> starts. If the thread is long enough, you'd see the same text quoted 
> over and over and over. All of that is not a problem within a single 
> patch email, but it adds up to lots of wasted time on all sides.

Yes

My concern about clarity is the history of the discussion.
When we post a new versions in GitHub, it's very hard to keep track
of the history.
As a maintainer, I need to see the history to understand what happened,
what we are waiting for, and what should be merged.


> And all of the above will not be a problem with a tool like 
> Gitlab/Github. There are "general" comments that can be used for general 
> discussion, and there are line-specific comments that can be used to 
> discuss certain sections of the patch. I've done this many times in many 
> reviews, and it works very well. Now, granted, I've never maintained an 
> entire repository like DPDK, so you may have a different perspective, 
> but i really don't see how long email chains have "clarity" that a 
> discussion thread with proper quoting, links to code, markdown syntax, 
> etc. doesn't.

You don't have discussion threading in GitHub. Is there?


> (for the record, i don't consider Gerrit to be a good tool because it 
> enforces a particular git workflow, one that is not at all compatible 
> with how our community works. GitLab, on the other hand, "just works" - 
> i'm assuming GitHub is very similar)
> 
> > 
> > There is a mailing list discussing workflow tooling:
> > 	https://lore.kernel.org/workflows/

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Jerin Jacob]

On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 25/05/2020 16:28, Burakov, Anatoly:
> > On 25-May-20 1:53 PM, Thomas Monjalon wrote:
> > > 25/05/2020 13:58, Jerin Jacob:
> > >> 25/05/2020 11:34, Morten Brørup:
> > >>> sending patches over an
> > >>> email as opposed to a well-integrated web interface workflow is so alien
> > >>> to most people that it definitely does discourage new contributions.
> > >>>
> > >>> I understand the advantages of mailing lists (vendor independence,
> > >>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
> > >>> for a while (we use those internally), going through DPDK mailing list
> > >>> and reviewing code over email fills me with existential dread, as the
> > >>> process feels so manual and 19th century to me.
> > >>
> > >> Agree. I had a difference in opinion when I was not using those tools.
> > >> My perspective changed after using Github and Gerrit etc.
> > >>
> > >> Github pull request and integrated public CI(Travis, Shippable ,
> > >> codecov) makes collaboration easy.
> > >> Currently, in patchwork, we can not assign a patch other than the set
> > >> of maintainers.
> > >> I think, it would help the review process if the more fine-grained
> > >> owner will be responsible for specific
> > >> patch set.
> > >
> > > The more fine-grain is achieved with Cc in mail.
> > > But I understand not everybody knows/wants/can configure correctly
> > > an email client. Emails are not easy for everybody, I agree.
> > >
> > > I use GitHub as well, and I really prefer the clarity of the mail threads.
> > > GitHub reviews tend to be line-focused, messy and not discussion-friendly.
> > > I think contribution quality would be worst if using GitHub.
> >
> > I have more experience with Gitlab than Github, but i really don't see
> > it that way.
> >
> > For one, reviewing in Gitlab makes it easier to see context in which
> > changes appear. I mean, obviously, you can download the patch, apply it,
> > and then do whatever you want with it in your editor/IDE, but it's just
> > so much faster to do it right in the browser. Reviewing things with
> > proper syntax highlighting and side-by-side diff with an option to see
> > more context really makes a huge difference and is that much faster.
>
> OK
>
>
> > I would also vehemently disagree with the "clarity" argument. There is
> > enforced minimum standard of clarity of discussion in a tool such as
> > Gitlab. I'm sure you noticed that some people top-post, some
> > bottom-post. Some will remove extraneous lines of patches while some
> > will leave on comment in a 10K line patch and leave the rest as is, in
> > quotes. Some people do weird quoting where they don't actually quote but
> > just copy text verbatim, making it hard to determine where the quote
> > starts. If the thread is long enough, you'd see the same text quoted
> > over and over and over. All of that is not a problem within a single
> > patch email, but it adds up to lots of wasted time on all sides.
>
> Yes
>
> My concern about clarity is the history of the discussion.
> When we post a new versions in GitHub, it's very hard to keep track
> of the history.
> As a maintainer, I need to see the history to understand what happened,
> what we are waiting for, and what should be merged.

IMO, The complete history is available per pull request URL.
I think, Github also email notification mechanism those to prefer to see
comments in the email too.

In addition to that, Bugzilla, patchwork, CI stuff all integrated into
one place.
I am quite impressed with vscode community collaboration.
https://github.com/Microsoft/vscode/pulls


>
>
> > And all of the above will not be a problem with a tool like
> > Gitlab/Github. There are "general" comments that can be used for general
> > discussion, and there are line-specific comments that can be used to
> > discuss certain sections of the patch. I've done this many times in many
> > reviews, and it works very well. Now, granted, I've never maintained an
> > entire repository like DPDK, so you may have a different perspective,
> > but i really don't see how long email chains have "clarity" that a
> > discussion thread with proper quoting, links to code, markdown syntax,
> > etc. doesn't.
>
> You don't have discussion threading in GitHub. Is there?
>
>
> > (for the record, i don't consider Gerrit to be a good tool because it
> > enforces a particular git workflow, one that is not at all compatible
> > with how our community works. GitLab, on the other hand, "just works" -
> > i'm assuming GitHub is very similar)
> >
> > >
> > > There is a mailing list discussing workflow tooling:
> > >     https://lore.kernel.org/workflows/
>
>
>

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Maxime Coquelin]

On 5/25/20 5:35 PM, Jerin Jacob wrote:
> On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 25/05/2020 16:28, Burakov, Anatoly:
>>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>>>> 25/05/2020 13:58, Jerin Jacob:
>>>>> 25/05/2020 11:34, Morten Brørup:
>>>>>> sending patches over an
>>>>>> email as opposed to a well-integrated web interface workflow is so alien
>>>>>> to most people that it definitely does discourage new contributions.
>>>>>>
>>>>>> I understand the advantages of mailing lists (vendor independence,
>>>>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>>>>> for a while (we use those internally), going through DPDK mailing list
>>>>>> and reviewing code over email fills me with existential dread, as the
>>>>>> process feels so manual and 19th century to me.
>>>>>
>>>>> Agree. I had a difference in opinion when I was not using those tools.
>>>>> My perspective changed after using Github and Gerrit etc.
>>>>>
>>>>> Github pull request and integrated public CI(Travis, Shippable ,
>>>>> codecov) makes collaboration easy.
>>>>> Currently, in patchwork, we can not assign a patch other than the set
>>>>> of maintainers.
>>>>> I think, it would help the review process if the more fine-grained
>>>>> owner will be responsible for specific
>>>>> patch set.
>>>>
>>>> The more fine-grain is achieved with Cc in mail.
>>>> But I understand not everybody knows/wants/can configure correctly
>>>> an email client. Emails are not easy for everybody, I agree.
>>>>
>>>> I use GitHub as well, and I really prefer the clarity of the mail threads.
>>>> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
>>>> I think contribution quality would be worst if using GitHub.
>>>
>>> I have more experience with Gitlab than Github, but i really don't see
>>> it that way.
>>>
>>> For one, reviewing in Gitlab makes it easier to see context in which
>>> changes appear. I mean, obviously, you can download the patch, apply it,
>>> and then do whatever you want with it in your editor/IDE, but it's just
>>> so much faster to do it right in the browser. Reviewing things with
>>> proper syntax highlighting and side-by-side diff with an option to see
>>> more context really makes a huge difference and is that much faster.
>>
>> OK
>>
>>
>>> I would also vehemently disagree with the "clarity" argument. There is
>>> enforced minimum standard of clarity of discussion in a tool such as
>>> Gitlab. I'm sure you noticed that some people top-post, some
>>> bottom-post. Some will remove extraneous lines of patches while some
>>> will leave on comment in a 10K line patch and leave the rest as is, in
>>> quotes. Some people do weird quoting where they don't actually quote but
>>> just copy text verbatim, making it hard to determine where the quote
>>> starts. If the thread is long enough, you'd see the same text quoted
>>> over and over and over. All of that is not a problem within a single
>>> patch email, but it adds up to lots of wasted time on all sides.
>>
>> Yes
>>
>> My concern about clarity is the history of the discussion.
>> When we post a new versions in GitHub, it's very hard to keep track
>> of the history.
>> As a maintainer, I need to see the history to understand what happened,
>> what we are waiting for, and what should be merged.
> 
> IMO, The complete history is available per pull request URL.
> I think, Github also email notification mechanism those to prefer to see
> comments in the email too.
> 
> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
> one place.
> I am quite impressed with vscode community collaboration.
> https://github.com/Microsoft/vscode/pulls

Out of curiosity, just checked the git history and I'm not that
impressed. For example last commit on the master branch:

https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148

Commit title: " Fix #98530 "
Commit message empty, no explanation on what the patch is doing.

Then, let's check the the issue it is pointed to:
https://github.com/microsoft/vscode/issues/98530

Issue is created 15 minutes before the patch is being merged. All that
done by the same contributor, without any review.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

On 25-May-20 4:52 PM, Maxime Coquelin wrote:
> 
> 
> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>> On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>
>>> 25/05/2020 16:28, Burakov, Anatoly:
>>>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>>>>> 25/05/2020 13:58, Jerin Jacob:
>>>>>> 25/05/2020 11:34, Morten Brørup:
>>>>>>> sending patches over an
>>>>>>> email as opposed to a well-integrated web interface workflow is so alien
>>>>>>> to most people that it definitely does discourage new contributions.
>>>>>>>
>>>>>>> I understand the advantages of mailing lists (vendor independence,
>>>>>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>>>>>> for a while (we use those internally), going through DPDK mailing list
>>>>>>> and reviewing code over email fills me with existential dread, as the
>>>>>>> process feels so manual and 19th century to me.
>>>>>>
>>>>>> Agree. I had a difference in opinion when I was not using those tools.
>>>>>> My perspective changed after using Github and Gerrit etc.
>>>>>>
>>>>>> Github pull request and integrated public CI(Travis, Shippable ,
>>>>>> codecov) makes collaboration easy.
>>>>>> Currently, in patchwork, we can not assign a patch other than the set
>>>>>> of maintainers.
>>>>>> I think, it would help the review process if the more fine-grained
>>>>>> owner will be responsible for specific
>>>>>> patch set.
>>>>>
>>>>> The more fine-grain is achieved with Cc in mail.
>>>>> But I understand not everybody knows/wants/can configure correctly
>>>>> an email client. Emails are not easy for everybody, I agree.
>>>>>
>>>>> I use GitHub as well, and I really prefer the clarity of the mail threads.
>>>>> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
>>>>> I think contribution quality would be worst if using GitHub.
>>>>
>>>> I have more experience with Gitlab than Github, but i really don't see
>>>> it that way.
>>>>
>>>> For one, reviewing in Gitlab makes it easier to see context in which
>>>> changes appear. I mean, obviously, you can download the patch, apply it,
>>>> and then do whatever you want with it in your editor/IDE, but it's just
>>>> so much faster to do it right in the browser. Reviewing things with
>>>> proper syntax highlighting and side-by-side diff with an option to see
>>>> more context really makes a huge difference and is that much faster.
>>>
>>> OK
>>>
>>>
>>>> I would also vehemently disagree with the "clarity" argument. There is
>>>> enforced minimum standard of clarity of discussion in a tool such as
>>>> Gitlab. I'm sure you noticed that some people top-post, some
>>>> bottom-post. Some will remove extraneous lines of patches while some
>>>> will leave on comment in a 10K line patch and leave the rest as is, in
>>>> quotes. Some people do weird quoting where they don't actually quote but
>>>> just copy text verbatim, making it hard to determine where the quote
>>>> starts. If the thread is long enough, you'd see the same text quoted
>>>> over and over and over. All of that is not a problem within a single
>>>> patch email, but it adds up to lots of wasted time on all sides.
>>>
>>> Yes
>>>
>>> My concern about clarity is the history of the discussion.
>>> When we post a new versions in GitHub, it's very hard to keep track
>>> of the history.
>>> As a maintainer, I need to see the history to understand what happened,
>>> what we are waiting for, and what should be merged.
>>
>> IMO, The complete history is available per pull request URL.
>> I think, Github also email notification mechanism those to prefer to see
>> comments in the email too.
>>
>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>> one place.
>> I am quite impressed with vscode community collaboration.
>> https://github.com/Microsoft/vscode/pulls
> 
> Out of curiosity, just checked the git history and I'm not that
> impressed. For example last commit on the master branch:
> 
> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
> 
> Commit title: " Fix #98530 "
> Commit message empty, no explanation on what the patch is doing.
> 
> Then, let's check the the issue it is pointed to:
> https://github.com/microsoft/vscode/issues/98530
> 
> Issue is created 15 minutes before the patch is being merged. All that
> done by the same contributor, without any review.
> 

Just because they do it wrong doesn't mean we can't do it right :) This 
says more about Microsoft's lack of process around VSCode than it does 
about Github the tool.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Maxime Coquelin]

On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>
>>
>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>> On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net>
>>> wrote:
>>>>
>>>> 25/05/2020 16:28, Burakov, Anatoly:
>>>>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>>>>>> 25/05/2020 13:58, Jerin Jacob:
>>>>>>> 25/05/2020 11:34, Morten Brørup:
>>>>>>>> sending patches over an
>>>>>>>> email as opposed to a well-integrated web interface workflow is
>>>>>>>> so alien
>>>>>>>> to most people that it definitely does discourage new
>>>>>>>> contributions.
>>>>>>>>
>>>>>>>> I understand the advantages of mailing lists (vendor independence,
>>>>>>>> universal compatibility, etc.), but after doing reviews in
>>>>>>>> Github/Gitlab
>>>>>>>> for a while (we use those internally), going through DPDK
>>>>>>>> mailing list
>>>>>>>> and reviewing code over email fills me with existential dread,
>>>>>>>> as the
>>>>>>>> process feels so manual and 19th century to me.
>>>>>>>
>>>>>>> Agree. I had a difference in opinion when I was not using those
>>>>>>> tools.
>>>>>>> My perspective changed after using Github and Gerrit etc.
>>>>>>>
>>>>>>> Github pull request and integrated public CI(Travis, Shippable ,
>>>>>>> codecov) makes collaboration easy.
>>>>>>> Currently, in patchwork, we can not assign a patch other than the
>>>>>>> set
>>>>>>> of maintainers.
>>>>>>> I think, it would help the review process if the more fine-grained
>>>>>>> owner will be responsible for specific
>>>>>>> patch set.
>>>>>>
>>>>>> The more fine-grain is achieved with Cc in mail.
>>>>>> But I understand not everybody knows/wants/can configure correctly
>>>>>> an email client. Emails are not easy for everybody, I agree.
>>>>>>
>>>>>> I use GitHub as well, and I really prefer the clarity of the mail
>>>>>> threads.
>>>>>> GitHub reviews tend to be line-focused, messy and not
>>>>>> discussion-friendly.
>>>>>> I think contribution quality would be worst if using GitHub.
>>>>>
>>>>> I have more experience with Gitlab than Github, but i really don't see
>>>>> it that way.
>>>>>
>>>>> For one, reviewing in Gitlab makes it easier to see context in which
>>>>> changes appear. I mean, obviously, you can download the patch,
>>>>> apply it,
>>>>> and then do whatever you want with it in your editor/IDE, but it's
>>>>> just
>>>>> so much faster to do it right in the browser. Reviewing things with
>>>>> proper syntax highlighting and side-by-side diff with an option to see
>>>>> more context really makes a huge difference and is that much faster.
>>>>
>>>> OK
>>>>
>>>>
>>>>> I would also vehemently disagree with the "clarity" argument. There is
>>>>> enforced minimum standard of clarity of discussion in a tool such as
>>>>> Gitlab. I'm sure you noticed that some people top-post, some
>>>>> bottom-post. Some will remove extraneous lines of patches while some
>>>>> will leave on comment in a 10K line patch and leave the rest as is, in
>>>>> quotes. Some people do weird quoting where they don't actually
>>>>> quote but
>>>>> just copy text verbatim, making it hard to determine where the quote
>>>>> starts. If the thread is long enough, you'd see the same text quoted
>>>>> over and over and over. All of that is not a problem within a single
>>>>> patch email, but it adds up to lots of wasted time on all sides.
>>>>
>>>> Yes
>>>>
>>>> My concern about clarity is the history of the discussion.
>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>> of the history.
>>>> As a maintainer, I need to see the history to understand what happened,
>>>> what we are waiting for, and what should be merged.
>>>
>>> IMO, The complete history is available per pull request URL.
>>> I think, Github also email notification mechanism those to prefer to see
>>> comments in the email too.
>>>
>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>> one place.
>>> I am quite impressed with vscode community collaboration.
>>> https://github.com/Microsoft/vscode/pulls
>>
>> Out of curiosity, just checked the git history and I'm not that
>> impressed. For example last commit on the master branch:
>>
>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>>
>>
>> Commit title: " Fix #98530 "
>> Commit message empty, no explanation on what the patch is doing.
>>
>> Then, let's check the the issue it is pointed to:
>> https://github.com/microsoft/vscode/issues/98530
>>
>> Issue is created 15 minutes before the patch is being merged. All that
>> done by the same contributor, without any review.
>>
> 
> Just because they do it wrong doesn't mean we can't do it right :) This
> says more about Microsoft's lack of process around VSCode than it does
> about Github the tool.
> 

True. I was just pointing out that is not the kind of process I would
personally want to adopt.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

On 25-May-20 5:04 PM, Maxime Coquelin wrote:
> 
> 
> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>
>>>
>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>> On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net>
>>>> wrote:
>>>>>
>>>>> 25/05/2020 16:28, Burakov, Anatoly:
>>>>>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>>>>>>> 25/05/2020 13:58, Jerin Jacob:
>>>>>>>> 25/05/2020 11:34, Morten Brørup:
>>>>>>>>> sending patches over an
>>>>>>>>> email as opposed to a well-integrated web interface workflow is
>>>>>>>>> so alien
>>>>>>>>> to most people that it definitely does discourage new
>>>>>>>>> contributions.
>>>>>>>>>
>>>>>>>>> I understand the advantages of mailing lists (vendor independence,
>>>>>>>>> universal compatibility, etc.), but after doing reviews in
>>>>>>>>> Github/Gitlab
>>>>>>>>> for a while (we use those internally), going through DPDK
>>>>>>>>> mailing list
>>>>>>>>> and reviewing code over email fills me with existential dread,
>>>>>>>>> as the
>>>>>>>>> process feels so manual and 19th century to me.
>>>>>>>>
>>>>>>>> Agree. I had a difference in opinion when I was not using those
>>>>>>>> tools.
>>>>>>>> My perspective changed after using Github and Gerrit etc.
>>>>>>>>
>>>>>>>> Github pull request and integrated public CI(Travis, Shippable ,
>>>>>>>> codecov) makes collaboration easy.
>>>>>>>> Currently, in patchwork, we can not assign a patch other than the
>>>>>>>> set
>>>>>>>> of maintainers.
>>>>>>>> I think, it would help the review process if the more fine-grained
>>>>>>>> owner will be responsible for specific
>>>>>>>> patch set.
>>>>>>>
>>>>>>> The more fine-grain is achieved with Cc in mail.
>>>>>>> But I understand not everybody knows/wants/can configure correctly
>>>>>>> an email client. Emails are not easy for everybody, I agree.
>>>>>>>
>>>>>>> I use GitHub as well, and I really prefer the clarity of the mail
>>>>>>> threads.
>>>>>>> GitHub reviews tend to be line-focused, messy and not
>>>>>>> discussion-friendly.
>>>>>>> I think contribution quality would be worst if using GitHub.
>>>>>>
>>>>>> I have more experience with Gitlab than Github, but i really don't see
>>>>>> it that way.
>>>>>>
>>>>>> For one, reviewing in Gitlab makes it easier to see context in which
>>>>>> changes appear. I mean, obviously, you can download the patch,
>>>>>> apply it,
>>>>>> and then do whatever you want with it in your editor/IDE, but it's
>>>>>> just
>>>>>> so much faster to do it right in the browser. Reviewing things with
>>>>>> proper syntax highlighting and side-by-side diff with an option to see
>>>>>> more context really makes a huge difference and is that much faster.
>>>>>
>>>>> OK
>>>>>
>>>>>
>>>>>> I would also vehemently disagree with the "clarity" argument. There is
>>>>>> enforced minimum standard of clarity of discussion in a tool such as
>>>>>> Gitlab. I'm sure you noticed that some people top-post, some
>>>>>> bottom-post. Some will remove extraneous lines of patches while some
>>>>>> will leave on comment in a 10K line patch and leave the rest as is, in
>>>>>> quotes. Some people do weird quoting where they don't actually
>>>>>> quote but
>>>>>> just copy text verbatim, making it hard to determine where the quote
>>>>>> starts. If the thread is long enough, you'd see the same text quoted
>>>>>> over and over and over. All of that is not a problem within a single
>>>>>> patch email, but it adds up to lots of wasted time on all sides.
>>>>>
>>>>> Yes
>>>>>
>>>>> My concern about clarity is the history of the discussion.
>>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>>> of the history.
>>>>> As a maintainer, I need to see the history to understand what happened,
>>>>> what we are waiting for, and what should be merged.
>>>>
>>>> IMO, The complete history is available per pull request URL.
>>>> I think, Github also email notification mechanism those to prefer to see
>>>> comments in the email too.
>>>>
>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>>> one place.
>>>> I am quite impressed with vscode community collaboration.
>>>> https://github.com/Microsoft/vscode/pulls
>>>
>>> Out of curiosity, just checked the git history and I'm not that
>>> impressed. For example last commit on the master branch:
>>>
>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>>>
>>>
>>> Commit title: " Fix #98530 "
>>> Commit message empty, no explanation on what the patch is doing.
>>>
>>> Then, let's check the the issue it is pointed to:
>>> https://github.com/microsoft/vscode/issues/98530
>>>
>>> Issue is created 15 minutes before the patch is being merged. All that
>>> done by the same contributor, without any review.
>>>
>>
>> Just because they do it wrong doesn't mean we can't do it right :) This
>> says more about Microsoft's lack of process around VSCode than it does
>> about Github the tool.
>>
> 
> True. I was just pointing out that is not the kind of process I would
> personally want to adopt.
> 

You won't find disagreement here, but this "process" is not due to the 
tool. You can just as well allow Thomas to merge stuff without any 
review because he has commit rights, no Github needed - and you would be 
faced with the same problem.

So, i don't think Jerin was suggesting that we degrade our merge/commit 
rules. Rather, the point was that (whatever you think of VSCode's 
review/merge process) there are a lot of pull requests and there is 
healthy community collaboration. I'm not saying we don't have that, 
obviously, but i have a suspicion that we'll get more of it if we lower 
the barrier for entry (not the barrier for merge!). I think there is a 
way to lower the secondary skill level needed to contribute to DPDK 
without lowering coding/merge standards with it.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Thomas Monjalon]

25/05/2020 18:09, Burakov, Anatoly:
> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
> > On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
> >> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
> >>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
> >>>> On May 25, 2020 Thomas Monjalon wrote:
> >>>>> My concern about clarity is the history of the discussion.
> >>>>> When we post a new versions in GitHub, it's very hard to keep track
> >>>>> of the history.
> >>>>> As a maintainer, I need to see the history to understand what happened,
> >>>>> what we are waiting for, and what should be merged.
> >>>>
> >>>> IMO, The complete history is available per pull request URL.
> >>>> I think, Github also email notification mechanism those to prefer to see
> >>>> comments in the email too.
> >>>>
> >>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
> >>>> one place.
> >>>> I am quite impressed with vscode community collaboration.
> >>>> https://github.com/Microsoft/vscode/pulls
> >>>
> >>> Out of curiosity, just checked the git history and I'm not that
> >>> impressed. For example last commit on the master branch:
> >>>
> >>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
> >>>
> >>>
> >>> Commit title: " Fix #98530 "
> >>> Commit message empty, no explanation on what the patch is doing.
> >>>
> >>> Then, let's check the the issue it is pointed to:
> >>> https://github.com/microsoft/vscode/issues/98530
> >>>
> >>> Issue is created 15 minutes before the patch is being merged. All that
> >>> done by the same contributor, without any review.
> >>>
> >>
> >> Just because they do it wrong doesn't mean we can't do it right :) This
> >> says more about Microsoft's lack of process around VSCode than it does
> >> about Github the tool.
> >>
> > 
> > True. I was just pointing out that is not the kind of process I would
> > personally want to adopt.
> > 
> 
> You won't find disagreement here, but this "process" is not due to the 
> tool. You can just as well allow Thomas to merge stuff without any 
> review because he has commit rights, no Github needed - and you would be 
> faced with the same problem.
> 
> So, i don't think Jerin was suggesting that we degrade our merge/commit 
> rules. Rather, the point was that (whatever you think of VSCode's 
> review/merge process) there are a lot of pull requests and there is 
> healthy community collaboration. I'm not saying we don't have that,

Yes, recent survey said the process was fine:
	http://mails.dpdk.org/archives/announce/2019-June/000268.html


> obviously, but i have a suspicion that we'll get more of it if we lower 
> the barrier for entry (not the barrier for merge!). I think there is a 
> way to lower the secondary skill level needed to contribute to DPDK 
> without lowering coding/merge standards with it.

About the barrier for entry, maybe it is not obvious because I don't
communicate a lot about it, but please be aware that I (and other
maintainers I think) are doing a lot of changes in newcomer patches
to avoid asking them knowing the whole process from the beginning.
Then frequent contributors get educated on the way.

I think the only real barrier we have is to sign the patch
with a real name and send an email to right list.
The ask for SoB real name is probably what started this thread
in Morten's mind. And the SoB requirement will *never* change.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 25/05/2020 18:09, Burakov, Anatoly:
>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>>>>> of the history.
>>>>>>> As a maintainer, I need to see the history to understand what happened,
>>>>>>> what we are waiting for, and what should be merged.
>>>>>> 
>>>>>> IMO, The complete history is available per pull request URL.
>>>>>> I think, Github also email notification mechanism those to prefer to see
>>>>>> comments in the email too.
>>>>>> 
>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>>>>> one place.
>>>>>> I am quite impressed with vscode community collaboration.
>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>> 
>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>> impressed. For example last commit on the master branch:
>>>>> 
>>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>>>>> 
>>>>> 
>>>>> Commit title: " Fix #98530 "
>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>> 
>>>>> Then, let's check the the issue it is pointed to:
>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>> 
>>>>> Issue is created 15 minutes before the patch is being merged. All that
>>>>> done by the same contributor, without any review.
>>>>> 
>>>> 
>>>> Just because they do it wrong doesn't mean we can't do it right :) This
>>>> says more about Microsoft's lack of process around VSCode than it does
>>>> about Github the tool.
>>>> 
>>> 
>>> True. I was just pointing out that is not the kind of process I would
>>> personally want to adopt.
>>> 
>> 
>> You won't find disagreement here, but this "process" is not due to the 
>> tool. You can just as well allow Thomas to merge stuff without any 
>> review because he has commit rights, no Github needed - and you would be 
>> faced with the same problem.
>> 
>> So, i don't think Jerin was suggesting that we degrade our merge/commit 
>> rules. Rather, the point was that (whatever you think of VSCode's 
>> review/merge process) there are a lot of pull requests and there is 
>> healthy community collaboration. I'm not saying we don't have that,
> 
> Yes, recent survey said the process was fine:
> 	http://mails.dpdk.org/archives/announce/2019-June/000268.html

IMO the survey is not a great tool for these types of things. The tech board and others that fully understand the process should decide. From my experience using Github or Gitlab is much easy and a single tool to submit patches to a project. Anatoly and others stated it very well and we should convert IMO, as I have always stated in the past.
> 
> 
>> obviously, but i have a suspicion that we'll get more of it if we lower 
>> the barrier for entry (not the barrier for merge!). I think there is a 
>> way to lower the secondary skill level needed to contribute to DPDK 
>> without lowering coding/merge standards with it.
> 
> About the barrier for entry, maybe it is not obvious because I don't
> communicate a lot about it, but please be aware that I (and other
> maintainers I think) are doing a lot of changes in newcomer patches
> to avoid asking them knowing the whole process from the beginning.
> Then frequent contributors get educated on the way.
> 
> I think the only real barrier we have is to sign the patch
> with a real name and send an email to right list.
> The ask for SoB real name is probably what started this thread
> in Morten's mind. And the SoB requirement will *never* change.
> 
> 

Would it not free up your time and energies by have the tools do most of the work. then you can focus on what matters the patch and developing more features? 

There is a reasons millions of developer use one of these two tools, instead of emailing patch around. We are a fairly small project compared to Linux Kernel and we are not developing code for the Linux kernel. Some of the process like coding standard is great, but the rest is just legacy IMO and not required to get the job done. Having tools to keep track of the minutia should free up more of your time for the real development.

Yes, it will be a learning curve for some and nailing down the process or rules for merge requests needs to be done.

All in all it will be a huge improvement for contributors.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Thomas Monjalon]

25/05/2020 18:57, Wiles, Keith:
> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 25/05/2020 18:09, Burakov, Anatoly:
> >> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
> >>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
> >>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
> >>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
> >>>>>> On May 25, 2020 Thomas Monjalon wrote:
> >>>>>>> My concern about clarity is the history of the discussion.
> >>>>>>> When we post a new versions in GitHub, it's very hard to keep track
> >>>>>>> of the history.
> >>>>>>> As a maintainer, I need to see the history to understand what happened,
> >>>>>>> what we are waiting for, and what should be merged.
> >>>>>> 
> >>>>>> IMO, The complete history is available per pull request URL.
> >>>>>> I think, Github also email notification mechanism those to prefer to see
> >>>>>> comments in the email too.
> >>>>>> 
> >>>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
> >>>>>> one place.
> >>>>>> I am quite impressed with vscode community collaboration.
> >>>>>> https://github.com/Microsoft/vscode/pulls
> >>>>> 
> >>>>> Out of curiosity, just checked the git history and I'm not that
> >>>>> impressed. For example last commit on the master branch:
> >>>>> 
> >>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
> >>>>> 
> >>>>> 
> >>>>> Commit title: " Fix #98530 "
> >>>>> Commit message empty, no explanation on what the patch is doing.
> >>>>> 
> >>>>> Then, let's check the the issue it is pointed to:
> >>>>> https://github.com/microsoft/vscode/issues/98530
> >>>>> 
> >>>>> Issue is created 15 minutes before the patch is being merged. All that
> >>>>> done by the same contributor, without any review.
> >>>>> 
> >>>> 
> >>>> Just because they do it wrong doesn't mean we can't do it right :) This
> >>>> says more about Microsoft's lack of process around VSCode than it does
> >>>> about Github the tool.
> >>>> 
> >>> 
> >>> True. I was just pointing out that is not the kind of process I would
> >>> personally want to adopt.
> >>> 
> >> 
> >> You won't find disagreement here, but this "process" is not due to the 
> >> tool. You can just as well allow Thomas to merge stuff without any 
> >> review because he has commit rights, no Github needed - and you would be 
> >> faced with the same problem.
> >> 
> >> So, i don't think Jerin was suggesting that we degrade our merge/commit 
> >> rules. Rather, the point was that (whatever you think of VSCode's 
> >> review/merge process) there are a lot of pull requests and there is 
> >> healthy community collaboration. I'm not saying we don't have that,
> > 
> > Yes, recent survey said the process was fine:
> > 	http://mails.dpdk.org/archives/announce/2019-June/000268.html
> 
> IMO the survey is not a great tool for these types of things. The tech board and others that fully understand the process should decide. From my experience using Github or Gitlab is much easy and a single tool to submit patches to a project. Anatoly and others stated it very well and we should convert IMO, as I have always stated in the past.
> > 
> > 
> >> obviously, but i have a suspicion that we'll get more of it if we lower 
> >> the barrier for entry (not the barrier for merge!). I think there is a 
> >> way to lower the secondary skill level needed to contribute to DPDK 
> >> without lowering coding/merge standards with it.
> > 
> > About the barrier for entry, maybe it is not obvious because I don't
> > communicate a lot about it, but please be aware that I (and other
> > maintainers I think) are doing a lot of changes in newcomer patches
> > to avoid asking them knowing the whole process from the beginning.
> > Then frequent contributors get educated on the way.
> > 
> > I think the only real barrier we have is to sign the patch
> > with a real name and send an email to right list.
> > The ask for SoB real name is probably what started this thread
> > in Morten's mind. And the SoB requirement will *never* change.
> 
> Would it not free up your time and energies by have the tools
> do most of the work. then you can focus on what matters the patch
> and developing more features?

No, GitHub is not helping to track root cause and define what should be backported.
It does not help to track Coverity issues.
It does not add Acks automatically (but patchwork does).
It does not send a notification when enough review is done (judgement needed here).
It does not split patches when different bugs are fixed.
etc...

But yes GitHub provides a beautiful interface,
and can help with reviews (even if not my taste).

One more thing I experience sometimes, GitHub requires only one account
for all hosted projects, so it helps leaving quick comments in projects
we are not familiar with.


> There is a reasons millions of developer use one of these two tools, instead of emailing patch around. We are a fairly small project compared to Linux Kernel and we are not developing code for the Linux kernel. Some of the process like coding standard is great, but the rest is just legacy IMO and not required to get the job done. Having tools to keep track of the minutia should free up more of your time for the real development.
> 
> Yes, it will be a learning curve for some and nailing down the process or rules for merge requests needs to be done.
> 
> All in all it will be a huge improvement for contributors.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 25, 2020, at 12:32 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 25/05/2020 18:57, Wiles, Keith:
>> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>>>>>>> of the history.
>>>>>>>>> As a maintainer, I need to see the history to understand what happened,
>>>>>>>>> what we are waiting for, and what should be merged.
>>>>>>>> 
>>>>>>>> IMO, The complete history is available per pull request URL.
>>>>>>>> I think, Github also email notification mechanism those to prefer to see
>>>>>>>> comments in the email too.
>>>>>>>> 
>>>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>>>>>>> one place.
>>>>>>>> I am quite impressed with vscode community collaboration.
>>>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>>>> 
>>>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>>>> impressed. For example last commit on the master branch:
>>>>>>> 
>>>>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>>>>>>> 
>>>>>>> 
>>>>>>> Commit title: " Fix #98530 "
>>>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>>>> 
>>>>>>> Then, let's check the the issue it is pointed to:
>>>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>>>> 
>>>>>>> Issue is created 15 minutes before the patch is being merged. All that
>>>>>>> done by the same contributor, without any review.
>>>>>>> 
>>>>>> 
>>>>>> Just because they do it wrong doesn't mean we can't do it right :) This
>>>>>> says more about Microsoft's lack of process around VSCode than it does
>>>>>> about Github the tool.
>>>>>> 
>>>>> 
>>>>> True. I was just pointing out that is not the kind of process I would
>>>>> personally want to adopt.
>>>>> 
>>>> 
>>>> You won't find disagreement here, but this "process" is not due to the 
>>>> tool. You can just as well allow Thomas to merge stuff without any 
>>>> review because he has commit rights, no Github needed - and you would be 
>>>> faced with the same problem.
>>>> 
>>>> So, i don't think Jerin was suggesting that we degrade our merge/commit 
>>>> rules. Rather, the point was that (whatever you think of VSCode's 
>>>> review/merge process) there are a lot of pull requests and there is 
>>>> healthy community collaboration. I'm not saying we don't have that,
>>> 
>>> Yes, recent survey said the process was fine:
>>> 	http://mails.dpdk.org/archives/announce/2019-June/000268.html
>> 
>> IMO the survey is not a great tool for these types of things. The tech board and others that fully understand the process should decide. From my experience using Github or Gitlab is much easy and a single tool to submit patches to a project. Anatoly and others stated it very well and we should convert IMO, as I have always stated in the past.
>>> 
>>> 
>>>> obviously, but i have a suspicion that we'll get more of it if we lower 
>>>> the barrier for entry (not the barrier for merge!). I think there is a 
>>>> way to lower the secondary skill level needed to contribute to DPDK 
>>>> without lowering coding/merge standards with it.
>>> 
>>> About the barrier for entry, maybe it is not obvious because I don't
>>> communicate a lot about it, but please be aware that I (and other
>>> maintainers I think) are doing a lot of changes in newcomer patches
>>> to avoid asking them knowing the whole process from the beginning.
>>> Then frequent contributors get educated on the way.
>>> 
>>> I think the only real barrier we have is to sign the patch
>>> with a real name and send an email to right list.
>>> The ask for SoB real name is probably what started this thread
>>> in Morten's mind. And the SoB requirement will *never* change.
>> 
>> Would it not free up your time and energies by have the tools
>> do most of the work. then you can focus on what matters the patch
>> and developing more features?
> 
> No, GitHub is not helping to track root cause and define what should be backported.
> It does not help to track Coverity issues.
> It does not add Acks automatically (but patchwork does).
> It does not send a notification when enough review is done (judgement needed here).
> It does not split patches when different bugs are fixed.
> etc...

Thanks for reading my emails and I am trying to help DPDK as a whole.

All of these seem to be supported by GitHub or GitLab in one way or another, but other more versed in these tools can correct me.

- We use Coverity and other tools attached to GitLab and they seem to be doing the job. I agree we will always find issues and these tools are not a complete answer and no tool is today.
- Acks can be done via the merge rules (at least in GitLab FWIW not used GitHub much).
- cherry-picking a merge request into multiple commit or different merge request appear to be supported.
- Notifications are part of the process with merge rules if I understand your comment.

We need to drag DPDK kicking and screaming into the year 2020 :-)

> 
> But yes GitHub provides a beautiful interface,
> and can help with reviews (even if not my taste).
> 
> One more thing I experience sometimes, GitHub requires only one account
> for all hosted projects, so it helps leaving quick comments in projects
> we are not familiar with.
> 
> 
>> There is a reasons millions of developer use one of these two tools, instead of emailing patch around. We are a fairly small project compared to Linux Kernel and we are not developing code for the Linux kernel. Some of the process like coding standard is great, but the rest is just legacy IMO and not required to get the job done. Having tools to keep track of the minutia should free up more of your time for the real development.
>> 
>> Yes, it will be a learning curve for some and nailing down the process or rules for merge requests needs to be done.
>> 
>> All in all it will be a huge improvement for contributors.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

On 25-May-20 8:26 PM, Tom Barbette wrote:
> 
> Le 25/05/2020 à 19:50, Wiles, Keith a écrit :
>>
>>> On May 25, 2020, at 12:32 PM, Thomas Monjalon <thomas@monjalon.net> 
>>> wrote:
>>>
>>> 25/05/2020 18:57, Wiles, Keith:
>>>> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> 
>>>> wrote:
>>>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>>>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>>>>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>>>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>>>>>> When we post a new versions in GitHub, it's very hard to keep 
>>>>>>>>>>> track
>>>>>>>>>>> of the history.
>>>>>>>>>>> As a maintainer, I need to see the history to understand what 
>>>>>>>>>>> happened,
>>>>>>>>>>> what we are waiting for, and what should be merged.
>>>>>>>>>> IMO, The complete history is available per pull request URL.
>>>>>>>>>> I think, Github also email notification mechanism those to 
>>>>>>>>>> prefer to see
>>>>>>>>>> comments in the email too.
>>>>>>>>>>
>>>>>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all 
>>>>>>>>>> integrated into
>>>>>>>>>> one place.
>>>>>>>>>> I am quite impressed with vscode community collaboration.
>>>>>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>>>>>> impressed. For example last commit on the master branch:
>>>>>>>>>
>>>>>>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Commit title: " Fix #98530 "
>>>>>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>>>>>>
>>>>>>>>> Then, let's check the the issue it is pointed to:
>>>>>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>>>>>>
>>>>>>>>> Issue is created 15 minutes before the patch is being merged. 
>>>>>>>>> All that
>>>>>>>>> done by the same contributor, without any review.
>>>>>>>>>
>>>>>>>> Just because they do it wrong doesn't mean we can't do it right 
>>>>>>>> :) This
>>>>>>>> says more about Microsoft's lack of process around VSCode than 
>>>>>>>> it does
>>>>>>>> about Github the tool.
>>>>>>>>
>>>>>>> True. I was just pointing out that is not the kind of process I 
>>>>>>> would
>>>>>>> personally want to adopt.
>>>>>>>
>>>>>> You won't find disagreement here, but this "process" is not due to 
>>>>>> the
>>>>>> tool. You can just as well allow Thomas to merge stuff without any
>>>>>> review because he has commit rights, no Github needed - and you 
>>>>>> would be
>>>>>> faced with the same problem.
>>>>>>
>>>>>> So, i don't think Jerin was suggesting that we degrade our 
>>>>>> merge/commit
>>>>>> rules. Rather, the point was that (whatever you think of VSCode's
>>>>>> review/merge process) there are a lot of pull requests and there is
>>>>>> healthy community collaboration. I'm not saying we don't have that,
>>>>> Yes, recent survey said the process was fine:
>>>>>     http://mails.dpdk.org/archives/announce/2019-June/000268.html
>>>> IMO the survey is not a great tool for these types of things. The 
>>>> tech board and others that fully understand the process should 
>>>> decide. From my experience using Github or Gitlab is much easy and a 
>>>> single tool to submit patches to a project. Anatoly and others 
>>>> stated it very well and we should convert IMO, as I have always 
>>>> stated in the past.
>>>>>
>>>>>> obviously, but i have a suspicion that we'll get more of it if we 
>>>>>> lower
>>>>>> the barrier for entry (not the barrier for merge!). I think there 
>>>>>> is a
>>>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>>>> without lowering coding/merge standards with it.
>>>>> About the barrier for entry, maybe it is not obvious because I don't
>>>>> communicate a lot about it, but please be aware that I (and other
>>>>> maintainers I think) are doing a lot of changes in newcomer patches
>>>>> to avoid asking them knowing the whole process from the beginning.
>>>>> Then frequent contributors get educated on the way.
>>>>>
>>>>> I think the only real barrier we have is to sign the patch
>>>>> with a real name and send an email to right list.
>>>>> The ask for SoB real name is probably what started this thread
>>>>> in Morten's mind. And the SoB requirement will *never* change.
>>>> Would it not free up your time and energies by have the tools
>>>> do most of the work. then you can focus on what matters the patch
>>>> and developing more features?
>>> No, GitHub is not helping to track root cause and define what should 
>>> be backported.
>>> It does not help to track Coverity issues.
>>> It does not add Acks automatically (but patchwork does).
>>> It does not send a notification when enough review is done (judgement 
>>> needed here).
>>> It does not split patches when different bugs are fixed.
>>> etc...
>> Thanks for reading my emails and I am trying to help DPDK as a whole.
>>
>> All of these seem to be supported by GitHub or GitLab in one way or 
>> another, but other more versed in these tools can correct me.
>>
>> - We use Coverity and other tools attached to GitLab and they seem to 
>> be doing the job. I agree we will always find issues and these tools 
>> are not a complete answer and no tool is today.
>> - Acks can be done via the merge rules (at least in GitLab FWIW not 
>> used GitHub much).
>> - cherry-picking a merge request into multiple commit or different 
>> merge request appear to be supported.
>> - Notifications are part of the process with merge rules if I 
>> understand your comment.
>>
>> We need to drag DPDK kicking and screaming into the year 2020 :-)
> 
> 
> Maybe we could find something that allows to "git push" to the 
> patchwork, where it kind of appears already as a github-like 
> discussion?  It doesn't miss a lot to enable writing/discussion from the 
> website directly.
> 
> Personnaly I've put a lot of efforts to fix simple comments, be sure 
> that I wrote "v2" here, sign-off there, cc-ed the right person, not mess 
> my dozen format-patch versions, changed only the cover letter, ... Quite 
> afraid of bothering that big mailing list for nothing (though It's true 
> people have gently helped). It would be much easier with a git push, a 
> fast online review of the diff, as on github/gitlab, and done. Also, 
> github allows online edits, and therefore allows "elders" to do small 
> fixes directly in the "patch". Some fixes are not worth the discussion 
> and the chain of mails. That's what I'm missing the most personnaly. 
> Doable from patchwork too I guess.

The problem is, we would then have to maintain these changes to 
patchwork :) So despite the pain of switching should we choose to do so, 
i think in the long run it's easier to switch to a solution that already 
does support all of this and is maintained by someone else.

> 
>>
>>> But yes GitHub provides a beautiful interface,
>>> and can help with reviews (even if not my taste).
>>>
>>> One more thing I experience sometimes, GitHub requires only one account
>>> for all hosted projects, so it helps leaving quick comments in projects
>>> we are not familiar with.
>>>
>>>
>>>> There is a reasons millions of developer use one of these two tools, 
>>>> instead of emailing patch around. We are a fairly small project 
>>>> compared to Linux Kernel and we are not developing code for the 
>>>> Linux kernel. Some of the process like coding standard is great, but 
>>>> the rest is just legacy IMO and not required to get the job done. 
>>>> Having tools to keep track of the minutia should free up more of 
>>>> your time for the real development.
>>>>
>>>> Yes, it will be a learning curve for some and nailing down the 
>>>> process or rules for merge requests needs to be done.
>>>>
>>>> All in all it will be a huge improvement for contributors.
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 26, 2020, at 4:33 AM, Burakov, Anatoly <anatoly.burakov@intel.com> wrote:
> 
> On 25-May-20 8:26 PM, Tom Barbette wrote:
>> Le 25/05/2020 à 19:50, Wiles, Keith a écrit :
>>> 
>>>> On May 25, 2020, at 12:32 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>> 
>>>> 25/05/2020 18:57, Wiles, Keith:
>>>>> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>>>>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>>>>>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>>>>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>>>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>>>>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>>>>>>>>>> of the history.
>>>>>>>>>>>> As a maintainer, I need to see the history to understand what happened,
>>>>>>>>>>>> what we are waiting for, and what should be merged.
>>>>>>>>>>> IMO, The complete history is available per pull request URL.
>>>>>>>>>>> I think, Github also email notification mechanism those to prefer to see
>>>>>>>>>>> comments in the email too.
>>>>>>>>>>> 
>>>>>>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>>>>>>>>>> one place.
>>>>>>>>>>> I am quite impressed with vscode community collaboration.
>>>>>>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>>>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>>>>>>> impressed. For example last commit on the master branch:
>>>>>>>>>> 
>>>>>>>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Commit title: " Fix #98530 "
>>>>>>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>>>>>>> 
>>>>>>>>>> Then, let's check the the issue it is pointed to:
>>>>>>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>>>>>>> 
>>>>>>>>>> Issue is created 15 minutes before the patch is being merged. All that
>>>>>>>>>> done by the same contributor, without any review.
>>>>>>>>>> 
>>>>>>>>> Just because they do it wrong doesn't mean we can't do it right :) This
>>>>>>>>> says more about Microsoft's lack of process around VSCode than it does
>>>>>>>>> about Github the tool.
>>>>>>>>> 
>>>>>>>> True. I was just pointing out that is not the kind of process I would
>>>>>>>> personally want to adopt.
>>>>>>>> 
>>>>>>> You won't find disagreement here, but this "process" is not due to the
>>>>>>> tool. You can just as well allow Thomas to merge stuff without any
>>>>>>> review because he has commit rights, no Github needed - and you would be
>>>>>>> faced with the same problem.
>>>>>>> 
>>>>>>> So, i don't think Jerin was suggesting that we degrade our merge/commit
>>>>>>> rules. Rather, the point was that (whatever you think of VSCode's
>>>>>>> review/merge process) there are a lot of pull requests and there is
>>>>>>> healthy community collaboration. I'm not saying we don't have that,
>>>>>> Yes, recent survey said the process was fine:
>>>>>>     http://mails.dpdk.org/archives/announce/2019-June/000268.html
>>>>> IMO the survey is not a great tool for these types of things. The tech board and others that fully understand the process should decide. From my experience using Github or Gitlab is much easy and a single tool to submit patches to a project. Anatoly and others stated it very well and we should convert IMO, as I have always stated in the past.
>>>>>> 
>>>>>>> obviously, but i have a suspicion that we'll get more of it if we lower
>>>>>>> the barrier for entry (not the barrier for merge!). I think there is a
>>>>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>>>>> without lowering coding/merge standards with it.
>>>>>> About the barrier for entry, maybe it is not obvious because I don't
>>>>>> communicate a lot about it, but please be aware that I (and other
>>>>>> maintainers I think) are doing a lot of changes in newcomer patches
>>>>>> to avoid asking them knowing the whole process from the beginning.
>>>>>> Then frequent contributors get educated on the way.
>>>>>> 
>>>>>> I think the only real barrier we have is to sign the patch
>>>>>> with a real name and send an email to right list.
>>>>>> The ask for SoB real name is probably what started this thread
>>>>>> in Morten's mind. And the SoB requirement will *never* change.
>>>>> Would it not free up your time and energies by have the tools
>>>>> do most of the work. then you can focus on what matters the patch
>>>>> and developing more features?
>>>> No, GitHub is not helping to track root cause and define what should be backported.
>>>> It does not help to track Coverity issues.
>>>> It does not add Acks automatically (but patchwork does).
>>>> It does not send a notification when enough review is done (judgement needed here).
>>>> It does not split patches when different bugs are fixed.
>>>> etc...
>>> Thanks for reading my emails and I am trying to help DPDK as a whole.
>>> 
>>> All of these seem to be supported by GitHub or GitLab in one way or another, but other more versed in these tools can correct me.
>>> 
>>> - We use Coverity and other tools attached to GitLab and they seem to be doing the job. I agree we will always find issues and these tools are not a complete answer and no tool is today.
>>> - Acks can be done via the merge rules (at least in GitLab FWIW not used GitHub much).
>>> - cherry-picking a merge request into multiple commit or different merge request appear to be supported.
>>> - Notifications are part of the process with merge rules if I understand your comment.
>>> 
>>> We need to drag DPDK kicking and screaming into the year 2020 :-)
>> Maybe we could find something that allows to "git push" to the patchwork, where it kind of appears already as a github-like discussion?  It doesn't miss a lot to enable writing/discussion from the website directly.
>> Personnaly I've put a lot of efforts to fix simple comments, be sure that I wrote "v2" here, sign-off there, cc-ed the right person, not mess my dozen format-patch versions, changed only the cover letter, ... Quite afraid of bothering that big mailing list for nothing (though It's true people have gently helped). It would be much easier with a git push, a fast online review of the diff, as on github/gitlab, and done. Also, github allows online edits, and therefore allows "elders" to do small fixes directly in the "patch". Some fixes are not worth the discussion and the chain of mails. That's what I'm missing the most personnaly. Doable from patchwork too I guess.
> 
> The problem is, we would then have to maintain these changes to patchwork :) So despite the pain of switching should we choose to do so, i think in the long run it's easier to switch to a solution that already does support all of this and is maintained by someone else.

+1, lets not keep patching our current process or adding yet another tool.
> 
>>> 
>>>> But yes GitHub provides a beautiful interface,
>>>> and can help with reviews (even if not my taste).
>>>> 
>>>> One more thing I experience sometimes, GitHub requires only one account
>>>> for all hosted projects, so it helps leaving quick comments in projects
>>>> we are not familiar with.
>>>> 
>>>> 
>>>>> There is a reasons millions of developer use one of these two tools, instead of emailing patch around. We are a fairly small project compared to Linux Kernel and we are not developing code for the Linux kernel. Some of the process like coding standard is great, but the rest is just legacy IMO and not required to get the job done. Having tools to keep track of the minutia should free up more of your time for the real development.
>>>>> 
>>>>> Yes, it will be a learning curve for some and nailing down the process or rules for merge requests needs to be done.
>>>>> 
>>>>> All in all it will be a huge improvement for contributors.
> 
> 
> -- 
> Thanks,
> Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 25, 2020, at 2:26 PM, Tom Barbette <barbette@kth.se> wrote:
> 
> 
> Le 25/05/2020 à 19:50, Wiles, Keith a écrit :
>> 
>>> On May 25, 2020, at 12:32 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 
>>> 25/05/2020 18:57, Wiles, Keith:
>>>> On May 25, 2020, at 11:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>>>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>>>>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>>>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>>>>>> When we post a new versions in GitHub, it's very hard to keep track
>>>>>>>>>>> of the history.
>>>>>>>>>>> As a maintainer, I need to see the history to understand what happened,
>>>>>>>>>>> what we are waiting for, and what should be merged.
>>>>>>>>>> IMO, The complete history is available per pull request URL.
>>>>>>>>>> I think, Github also email notification mechanism those to prefer to see
>>>>>>>>>> comments in the email too.
>>>>>>>>>> 
>>>>>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all integrated into
>>>>>>>>>> one place.
>>>>>>>>>> I am quite impressed with vscode community collaboration.
>>>>>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>>>>>> impressed. For example last commit on the master branch:
>>>>>>>>> 
>>>>>>>>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Commit title: " Fix #98530 "
>>>>>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>>>>>> 
>>>>>>>>> Then, let's check the the issue it is pointed to:
>>>>>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>>>>>> 
>>>>>>>>> Issue is created 15 minutes before the patch is being merged. All that
>>>>>>>>> done by the same contributor, without any review.
>>>>>>>>> 
>>>>>>>> Just because they do it wrong doesn't mean we can't do it right :) This
>>>>>>>> says more about Microsoft's lack of process around VSCode than it does
>>>>>>>> about Github the tool.
>>>>>>>> 
>>>>>>> True. I was just pointing out that is not the kind of process I would
>>>>>>> personally want to adopt.
>>>>>>> 
>>>>>> You won't find disagreement here, but this "process" is not due to the
>>>>>> tool. You can just as well allow Thomas to merge stuff without any
>>>>>> review because he has commit rights, no Github needed - and you would be
>>>>>> faced with the same problem.
>>>>>> 
>>>>>> So, i don't think Jerin was suggesting that we degrade our merge/commit
>>>>>> rules. Rather, the point was that (whatever you think of VSCode's
>>>>>> review/merge process) there are a lot of pull requests and there is
>>>>>> healthy community collaboration. I'm not saying we don't have that,
>>>>> Yes, recent survey said the process was fine:
>>>>> 	http://mails.dpdk.org/archives/announce/2019-June/000268.html
>>>> IMO the survey is not a great tool for these types of things. The tech board and others that fully understand the process should decide. From my experience using Github or Gitlab is much easy and a single tool to submit patches to a project. Anatoly and others stated it very well and we should convert IMO, as I have always stated in the past.
>>>>> 
>>>>>> obviously, but i have a suspicion that we'll get more of it if we lower
>>>>>> the barrier for entry (not the barrier for merge!). I think there is a
>>>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>>>> without lowering coding/merge standards with it.
>>>>> About the barrier for entry, maybe it is not obvious because I don't
>>>>> communicate a lot about it, but please be aware that I (and other
>>>>> maintainers I think) are doing a lot of changes in newcomer patches
>>>>> to avoid asking them knowing the whole process from the beginning.
>>>>> Then frequent contributors get educated on the way.
>>>>> 
>>>>> I think the only real barrier we have is to sign the patch
>>>>> with a real name and send an email to right list.
>>>>> The ask for SoB real name is probably what started this thread
>>>>> in Morten's mind. And the SoB requirement will *never* change.
>>>> Would it not free up your time and energies by have the tools
>>>> do most of the work. then you can focus on what matters the patch
>>>> and developing more features?
>>> No, GitHub is not helping to track root cause and define what should be backported.
>>> It does not help to track Coverity issues.
>>> It does not add Acks automatically (but patchwork does).
>>> It does not send a notification when enough review is done (judgement needed here).
>>> It does not split patches when different bugs are fixed.
>>> etc...
>> Thanks for reading my emails and I am trying to help DPDK as a whole.
>> 
>> All of these seem to be supported by GitHub or GitLab in one way or another, but other more versed in these tools can correct me.
>> 
>> - We use Coverity and other tools attached to GitLab and they seem to be doing the job. I agree we will always find issues and these tools are not a complete answer and no tool is today.
>> - Acks can be done via the merge rules (at least in GitLab FWIW not used GitHub much).
>> - cherry-picking a merge request into multiple commit or different merge request appear to be supported.
>> - Notifications are part of the process with merge rules if I understand your comment.
>> 
>> We need to drag DPDK kicking and screaming into the year 2020 :-)
> 
> 
> Maybe we could find something that allows to "git push" to the patchwork, where it kind of appears already as a github-like discussion?  It doesn't miss a lot to enable writing/discussion from the website directly.
> 
> Personnaly I've put a lot of efforts to fix simple comments, be sure that I wrote "v2" here, sign-off there, cc-ed the right person, not mess my dozen format-patch versions, changed only the cover letter, ...  Quite afraid of bothering that big mailing list for nothing (though It's true people have gently helped). It would be much easier with a git push, a fast online review of the diff, as on github/gitlab, and done. Also, github allows online edits, and therefore allows "elders" to do small fixes directly in the "patch". Some fixes are not worth the discussion and the chain of mails. That's what I'm missing the most personnaly. Doable from patchwork too I guess.

We could add new tools or modified existing tools like patchwork to meet our needs the problem is we are then just duplicating GitHub or GitLab. We should move to one of these and not continue to try and make it work. I understand the need to make changes to our current setup, but in the long run maybe duplicating these other tools that have a fairly large team (I think) to improve and maintain them.
> 
>> 
>>> But yes GitHub provides a beautiful interface,
>>> and can help with reviews (even if not my taste).
>>> 
>>> One more thing I experience sometimes, GitHub requires only one account
>>> for all hosted projects, so it helps leaving quick comments in projects
>>> we are not familiar with.
>>> 
>>> 
>>>> There is a reasons millions of developer use one of these two tools, instead of emailing patch around. We are a fairly small project compared to Linux Kernel and we are not developing code for the Linux kernel. Some of the process like coding standard is great, but the rest is just legacy IMO and not required to get the job done. Having tools to keep track of the minutia should free up more of your time for the real development.
>>>> 
>>>> Yes, it will be a learning curve for some and nailing down the process or rules for merge requests needs to be done.
>>>> 
>>>> All in all it will be a huge improvement for contributors.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Morten Brørup]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, May 25, 2020 6:29 PM
> 
> 25/05/2020 18:09, Burakov, Anatoly:
> > On 25-May-20 5:04 PM, Maxime Coquelin wrote:
> > > On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
> > >> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
> > >>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
> > >>>> On May 25, 2020 Thomas Monjalon wrote:
> > >>>>> My concern about clarity is the history of the discussion.
> > >>>>> When we post a new versions in GitHub, it's very hard to keep
> track
> > >>>>> of the history.
> > >>>>> As a maintainer, I need to see the history to understand what
> happened,
> > >>>>> what we are waiting for, and what should be merged.
> > >>>>
> > >>>> IMO, The complete history is available per pull request URL.
> > >>>> I think, Github also email notification mechanism those to
> prefer to see
> > >>>> comments in the email too.
> > >>>>
> > >>>> In addition to that, Bugzilla, patchwork, CI stuff all
> integrated into
> > >>>> one place.
> > >>>> I am quite impressed with vscode community collaboration.
> > >>>> https://github.com/Microsoft/vscode/pulls
> > >>>
> > >>> Out of curiosity, just checked the git history and I'm not that
> > >>> impressed. For example last commit on the master branch:
> > >>>
> > >>>
> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb74
> 46b3915d6148
> > >>>
> > >>>
> > >>> Commit title: " Fix #98530 "
> > >>> Commit message empty, no explanation on what the patch is doing.
> > >>>
> > >>> Then, let's check the the issue it is pointed to:
> > >>> https://github.com/microsoft/vscode/issues/98530
> > >>>
> > >>> Issue is created 15 minutes before the patch is being merged. All
> that
> > >>> done by the same contributor, without any review.
> > >>>
> > >>
> > >> Just because they do it wrong doesn't mean we can't do it right :)
> This
> > >> says more about Microsoft's lack of process around VSCode than it
> does
> > >> about Github the tool.
> > >>
> > >
> > > True. I was just pointing out that is not the kind of process I
> would
> > > personally want to adopt.
> > >
> >
> > You won't find disagreement here, but this "process" is not due to
> the
> > tool. You can just as well allow Thomas to merge stuff without any
> > review because he has commit rights, no Github needed - and you would
> be
> > faced with the same problem.
> >
> > So, i don't think Jerin was suggesting that we degrade our
> merge/commit
> > rules. Rather, the point was that (whatever you think of VSCode's
> > review/merge process) there are a lot of pull requests and there is
> > healthy community collaboration. I'm not saying we don't have that,
> 
> Yes, recent survey said the process was fine:
> 	http://mails.dpdk.org/archives/announce/2019-June/000268.html
> 
> 
> > obviously, but i have a suspicion that we'll get more of it if we
> lower
> > the barrier for entry (not the barrier for merge!). I think there is
> a
> > way to lower the secondary skill level needed to contribute to DPDK
> > without lowering coding/merge standards with it.

That is exactly what I am asking for: Lowering the barrier and increasing the feeling of success for newcomers. (The barrier for merge is probably fine; I'll leave that discussion to the maintainers.)

> 
> About the barrier for entry, maybe it is not obvious because I don't
> communicate a lot about it, but please be aware that I (and other
> maintainers I think) are doing a lot of changes in newcomer patches
> to avoid asking them knowing the whole process from the beginning.
> Then frequent contributors get educated on the way.

Great! I wish that every developer would think and behave this way.

> 
> I think the only real barrier we have is to sign the patch
> with a real name and send an email to right list.
> The ask for SoB real name is probably what started this thread
> in Morten's mind. And the SoB requirement will *never* change.

The incorrect Signed-off-by might be the only hard barrier (which we cannot avoid). But that did not trigger me.

I was raising the discussion to bring attention to soft barriers for contributors. What triggered me was the request to split the patch into multiple patches; a kind of feedback I have seen before. For an experienced git user, this is probably very easy, but for a git newbie (like myself), it basically means starting all over and trying to figure out the right set of git commands to do this, which can be perceived as a difficult task requiring a lot of effort.

Perhaps we could supplement the Contributor Guidelines with a set of cookbooks for different steps in the contribution process, so reviewers can be refer newcomers to the relevant of these as part of the feedback. Just like any professional customer support team has a set of canned answers ready for common customer issues. (Please note: I am not suggesting adding an AI/ML chat bot reviewer to the mailing list!)

The amount of Contributor Guideline documentation is also a balance... it must be long enough to contain the relevant information to get going, but short enough for newcomers to bother reading it.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Thomas Monjalon]

25/05/2020 20:44, Morten Brørup:
> From: Thomas Monjalon
> > 25/05/2020 18:09, Burakov, Anatoly:
> > > obviously, but i have a suspicion that we'll get more of it if we
> > lower
> > > the barrier for entry (not the barrier for merge!). I think there is
> > a
> > > way to lower the secondary skill level needed to contribute to DPDK
> > > without lowering coding/merge standards with it.
> 
> That is exactly what I am asking for: Lowering the barrier and increasing the feeling of success for newcomers. (The barrier for merge is probably fine; I'll leave that discussion to the maintainers.)

I understand.


> > About the barrier for entry, maybe it is not obvious because I don't
> > communicate a lot about it, but please be aware that I (and other
> > maintainers I think) are doing a lot of changes in newcomer patches
> > to avoid asking them knowing the whole process from the beginning.
> > Then frequent contributors get educated on the way.
> 
> Great! I wish that every developer would think and behave this way.
> 
> > 
> > I think the only real barrier we have is to sign the patch
> > with a real name and send an email to right list.
> > The ask for SoB real name is probably what started this thread
> > in Morten's mind. And the SoB requirement will *never* change.
> 
> The incorrect Signed-off-by might be the only hard barrier (which we cannot avoid). But that did not trigger me.
> 
> I was raising the discussion to bring attention to soft barriers for contributors. What triggered me was the request to split the patch into multiple patches; a kind of feedback I have seen before. For an experienced git user, this is probably very easy, but for a git newbie (like myself), it basically means starting all over and trying to figure out the right set of git commands to do this, which can be perceived as a difficult task requiring a lot of effort.

Yes I am aware about this difficulty.
It is basically knowing git-reset and git-add -p.
I agree a cookbook for this kind of thing is required.

I would like to do the split for newcomers,
but we need also to validate the explanations of each commit.
A solution in such case is to send the split so the newbie can just
fill what is missing.
This kind of workflow is really what we should look at improving.


> Perhaps we could supplement the Contributor Guidelines with a set of cookbooks for different steps in the contribution process, so reviewers can be refer newcomers to the relevant of these as part of the feedback. Just like any professional customer support team has a set of canned answers ready for common customer issues. (Please note: I am not suggesting adding an AI/ML chat bot reviewer to the mailing list!)

OK


> The amount of Contributor Guideline documentation is also a balance... it must be long enough to contain the relevant information to get going, but short enough for newcomers to bother reading it.

Yes, we need short intros and long explanations when really needed.
It is touching another issue: we lack some documentation love.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Tom Barbette]

Le 25/05/2020 à 22:34, Thomas Monjalon a écrit :
> 25/05/2020 20:44, Morten Brørup:
>> From: Thomas Monjalon
>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>> obviously, but i have a suspicion that we'll get more of it if we
>>> lower
>>>> the barrier for entry (not the barrier for merge!). I think there is
>>> a
>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>> without lowering coding/merge standards with it.
>>
>> That is exactly what I am asking for: Lowering the barrier and increasing the feeling of success for newcomers. (The barrier for merge is probably fine; I'll leave that discussion to the maintainers.)
> 
> I understand.
> 
> 
>>> About the barrier for entry, maybe it is not obvious because I don't
>>> communicate a lot about it, but please be aware that I (and other
>>> maintainers I think) are doing a lot of changes in newcomer patches
>>> to avoid asking them knowing the whole process from the beginning.
>>> Then frequent contributors get educated on the way.
>>
>> Great! I wish that every developer would think and behave this way.
>>
>>>
>>> I think the only real barrier we have is to sign the patch
>>> with a real name and send an email to right list.
>>> The ask for SoB real name is probably what started this thread
>>> in Morten's mind. And the SoB requirement will *never* change.
>>
>> The incorrect Signed-off-by might be the only hard barrier (which we cannot avoid). But that did not trigger me.
>>
>> I was raising the discussion to bring attention to soft barriers for contributors. What triggered me was the request to split the patch into multiple patches; a kind of feedback I have seen before. For an experienced git user, this is probably very easy, but for a git newbie (like myself), it basically means starting all over and trying to figure out the right set of git commands to do this, which can be perceived as a difficult task requiring a lot of effort.
> 
> Yes I am aware about this difficulty.
> It is basically knowing git-reset and git-add -p.
> I agree a cookbook for this kind of thing is required.
> 
> I would like to do the split for newcomers,
> but we need also to validate the explanations of each commit.
> A solution in such case is to send the split so the newbie can just
> fill what is missing.
> This kind of workflow is really what we should look at improving.
> 
> 
>> Perhaps we could supplement the Contributor Guidelines with a set of cookbooks for different steps in the contribution process, so reviewers can be refer newcomers to the relevant of these as part of the feedback. Just like any professional customer support team has a set of canned answers ready for common customer issues. (Please note: I am not suggesting adding an AI/ML chat bot reviewer to the mailing list!)
> 
> OK
> 
> 
>> The amount of Contributor Guideline documentation is also a balance... it must be long enough to contain the relevant information to get going, but short enough for newcomers to bother reading it.
> 
> Yes, we need short intros and long explanations when really needed.
> It is touching another issue: we lack some documentation love.
> 
> 


Maybe we could find something that allows to "git push" to the 
patchwork, where it kind of appears already as a github-like discussion? 
  It doesn't miss a lot to enable writing from the website directly 
(basically auto-email).

Personnaly I've put a lot of efforts to fix simple comments, be sure 
that I wrote "v2" here, sign-off there, cc-ed the right person, not mess 
my the format-patch versions, changed only the cover letter, ... Quite 
afraid of bothering that big mailing list for nothing.

I'm infrequent enough to have te re-learn every time basically. It would 
be much easier with a git push, a fast online review of the diff, as on 
github/gitlab, and done. Also, those allow online edits, and therefore 
allows "elders" to do small fixes directly in the "patch". Some fixes 
are not worth the discussion and the chain of mails. That's what I'm 
missing the most personnaly.

Tom

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Maxime Coquelin]

On 5/26/20 9:06 AM, Tom Barbette wrote:
> Le 25/05/2020 à 22:34, Thomas Monjalon a écrit :
>> 25/05/2020 20:44, Morten Brørup:
>>> From: Thomas Monjalon
>>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>>> obviously, but i have a suspicion that we'll get more of it if we
>>>> lower
>>>>> the barrier for entry (not the barrier for merge!). I think there is
>>>> a
>>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>>> without lowering coding/merge standards with it.
>>>
>>> That is exactly what I am asking for: Lowering the barrier and
>>> increasing the feeling of success for newcomers. (The barrier for
>>> merge is probably fine; I'll leave that discussion to the maintainers.)
>>
>> I understand.
>>
>>
>>>> About the barrier for entry, maybe it is not obvious because I don't
>>>> communicate a lot about it, but please be aware that I (and other
>>>> maintainers I think) are doing a lot of changes in newcomer patches
>>>> to avoid asking them knowing the whole process from the beginning.
>>>> Then frequent contributors get educated on the way.
>>>
>>> Great! I wish that every developer would think and behave this way.
>>>
>>>>
>>>> I think the only real barrier we have is to sign the patch
>>>> with a real name and send an email to right list.
>>>> The ask for SoB real name is probably what started this thread
>>>> in Morten's mind. And the SoB requirement will *never* change.
>>>
>>> The incorrect Signed-off-by might be the only hard barrier (which we
>>> cannot avoid). But that did not trigger me.
>>>
>>> I was raising the discussion to bring attention to soft barriers for
>>> contributors. What triggered me was the request to split the patch
>>> into multiple patches; a kind of feedback I have seen before. For an
>>> experienced git user, this is probably very easy, but for a git
>>> newbie (like myself), it basically means starting all over and trying
>>> to figure out the right set of git commands to do this, which can be
>>> perceived as a difficult task requiring a lot of effort.
>>
>> Yes I am aware about this difficulty.
>> It is basically knowing git-reset and git-add -p.
>> I agree a cookbook for this kind of thing is required.
>>
>> I would like to do the split for newcomers,
>> but we need also to validate the explanations of each commit.
>> A solution in such case is to send the split so the newbie can just
>> fill what is missing.
>> This kind of workflow is really what we should look at improving.
>>
>>
>>> Perhaps we could supplement the Contributor Guidelines with a set of
>>> cookbooks for different steps in the contribution process, so
>>> reviewers can be refer newcomers to the relevant of these as part of
>>> the feedback. Just like any professional customer support team has a
>>> set of canned answers ready for common customer issues. (Please note:
>>> I am not suggesting adding an AI/ML chat bot reviewer to the mailing
>>> list!)
>>
>> OK
>>
>>
>>> The amount of Contributor Guideline documentation is also a
>>> balance... it must be long enough to contain the relevant information
>>> to get going, but short enough for newcomers to bother reading it.
>>
>> Yes, we need short intros and long explanations when really needed.
>> It is touching another issue: we lack some documentation love.
>>
>>
> 
> 
> Maybe we could find something that allows to "git push" to the
> patchwork, where it kind of appears already as a github-like discussion?
>  It doesn't miss a lot to enable writing from the website directly
> (basically auto-email).
> 
> Personnaly I've put a lot of efforts to fix simple comments, be sure
> that I wrote "v2" here, sign-off there, cc-ed the right person, not mess
> my the format-patch versions, changed only the cover letter, ... Quite
> afraid of bothering that big mailing list for nothing.

Maybe using git-publish would help here:
https://github.com/stefanha/git-publish

Using the simple git-puslish command, it manages revisions
automatically, open an editor for the cover letter, can run some scripts
to add proper maintainers, and hook available to run basic checks,
etc...

We could add a .gitpublish file to automate adding right maintainers
depending on the branch, etc...

For example, for Qemu the .gitpublish file looks like this:
https://github.com/qemu/qemu/blob/master/.gitpublish

> I'm infrequent enough to have te re-learn every time basically. It would
> be much easier with a git push, a fast online review of the diff, as on
> github/gitlab, and done. Also, those allow online edits, and therefore
> allows "elders" to do small fixes directly in the "patch". Some fixes
> are not worth the discussion and the chain of mails. That's what I'm
> missing the most personnaly.
> 
> Tom
> 

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Burakov, Anatoly]

On 26-May-20 8:31 AM, Maxime Coquelin wrote:
> 
> 
> On 5/26/20 9:06 AM, Tom Barbette wrote:
>> Le 25/05/2020 à 22:34, Thomas Monjalon a écrit :
>>> 25/05/2020 20:44, Morten Brørup:
>>>> From: Thomas Monjalon
>>>>> 25/05/2020 18:09, Burakov, Anatoly:
>>>>>> obviously, but i have a suspicion that we'll get more of it if we
>>>>> lower
>>>>>> the barrier for entry (not the barrier for merge!). I think there is
>>>>> a
>>>>>> way to lower the secondary skill level needed to contribute to DPDK
>>>>>> without lowering coding/merge standards with it.
>>>>
>>>> That is exactly what I am asking for: Lowering the barrier and
>>>> increasing the feeling of success for newcomers. (The barrier for
>>>> merge is probably fine; I'll leave that discussion to the maintainers.)
>>>
>>> I understand.
>>>
>>>
>>>>> About the barrier for entry, maybe it is not obvious because I don't
>>>>> communicate a lot about it, but please be aware that I (and other
>>>>> maintainers I think) are doing a lot of changes in newcomer patches
>>>>> to avoid asking them knowing the whole process from the beginning.
>>>>> Then frequent contributors get educated on the way.
>>>>
>>>> Great! I wish that every developer would think and behave this way.
>>>>
>>>>>
>>>>> I think the only real barrier we have is to sign the patch
>>>>> with a real name and send an email to right list.
>>>>> The ask for SoB real name is probably what started this thread
>>>>> in Morten's mind. And the SoB requirement will *never* change.
>>>>
>>>> The incorrect Signed-off-by might be the only hard barrier (which we
>>>> cannot avoid). But that did not trigger me.
>>>>
>>>> I was raising the discussion to bring attention to soft barriers for
>>>> contributors. What triggered me was the request to split the patch
>>>> into multiple patches; a kind of feedback I have seen before. For an
>>>> experienced git user, this is probably very easy, but for a git
>>>> newbie (like myself), it basically means starting all over and trying
>>>> to figure out the right set of git commands to do this, which can be
>>>> perceived as a difficult task requiring a lot of effort.
>>>
>>> Yes I am aware about this difficulty.
>>> It is basically knowing git-reset and git-add -p.
>>> I agree a cookbook for this kind of thing is required.
>>>
>>> I would like to do the split for newcomers,
>>> but we need also to validate the explanations of each commit.
>>> A solution in such case is to send the split so the newbie can just
>>> fill what is missing.
>>> This kind of workflow is really what we should look at improving.
>>>
>>>
>>>> Perhaps we could supplement the Contributor Guidelines with a set of
>>>> cookbooks for different steps in the contribution process, so
>>>> reviewers can be refer newcomers to the relevant of these as part of
>>>> the feedback. Just like any professional customer support team has a
>>>> set of canned answers ready for common customer issues. (Please note:
>>>> I am not suggesting adding an AI/ML chat bot reviewer to the mailing
>>>> list!)
>>>
>>> OK
>>>
>>>
>>>> The amount of Contributor Guideline documentation is also a
>>>> balance... it must be long enough to contain the relevant information
>>>> to get going, but short enough for newcomers to bother reading it.
>>>
>>> Yes, we need short intros and long explanations when really needed.
>>> It is touching another issue: we lack some documentation love.
>>>
>>>
>>
>>
>> Maybe we could find something that allows to "git push" to the
>> patchwork, where it kind of appears already as a github-like discussion?
>>   It doesn't miss a lot to enable writing from the website directly
>> (basically auto-email).
>>
>> Personnaly I've put a lot of efforts to fix simple comments, be sure
>> that I wrote "v2" here, sign-off there, cc-ed the right person, not mess
>> my the format-patch versions, changed only the cover letter, ... Quite
>> afraid of bothering that big mailing list for nothing.
> 
> Maybe using git-publish would help here:
> https://github.com/stefanha/git-publish
> 
> Using the simple git-puslish command, it manages revisions
> automatically, open an editor for the cover letter, can run some scripts
> to add proper maintainers, and hook available to run basic checks,
> etc...

Good to see that i've reinvented the wheel yet again, as this looks 
almost exactly like the set of scripts i wrote for myself to automate 
patch submission :D

> 
> We could add a .gitpublish file to automate adding right maintainers
> depending on the branch, etc...
> 
> For example, for Qemu the .gitpublish file looks like this:
> https://github.com/qemu/qemu/blob/master/.gitpublish
> 
>> I'm infrequent enough to have te re-learn every time basically. It would
>> be much easier with a git push, a fast online review of the diff, as on
>> github/gitlab, and done. Also, those allow online edits, and therefore
>> allows "elders" to do small fixes directly in the "patch". Some fixes
>> are not worth the discussion and the chain of mails. That's what I'm
>> missing the most personnaly.
>>
>> Tom
>>
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Burakov, Anatoly]

On 25-May-20 7:44 PM, Morten Brørup wrote:
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>> Sent: Monday, May 25, 2020 6:29 PM
>>
>> 25/05/2020 18:09, Burakov, Anatoly:
>>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
>>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
>>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
>>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
>>>>>>> On May 25, 2020 Thomas Monjalon wrote:
>>>>>>>> My concern about clarity is the history of the discussion.
>>>>>>>> When we post a new versions in GitHub, it's very hard to keep
>> track
>>>>>>>> of the history.
>>>>>>>> As a maintainer, I need to see the history to understand what
>> happened,
>>>>>>>> what we are waiting for, and what should be merged.
>>>>>>>
>>>>>>> IMO, The complete history is available per pull request URL.
>>>>>>> I think, Github also email notification mechanism those to
>> prefer to see
>>>>>>> comments in the email too.
>>>>>>>
>>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all
>> integrated into
>>>>>>> one place.
>>>>>>> I am quite impressed with vscode community collaboration.
>>>>>>> https://github.com/Microsoft/vscode/pulls
>>>>>>
>>>>>> Out of curiosity, just checked the git history and I'm not that
>>>>>> impressed. For example last commit on the master branch:
>>>>>>
>>>>>>
>> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb74
>> 46b3915d6148
>>>>>>
>>>>>>
>>>>>> Commit title: " Fix #98530 "
>>>>>> Commit message empty, no explanation on what the patch is doing.
>>>>>>
>>>>>> Then, let's check the the issue it is pointed to:
>>>>>> https://github.com/microsoft/vscode/issues/98530
>>>>>>
>>>>>> Issue is created 15 minutes before the patch is being merged. All
>> that
>>>>>> done by the same contributor, without any review.
>>>>>>
>>>>>
>>>>> Just because they do it wrong doesn't mean we can't do it right :)
>> This
>>>>> says more about Microsoft's lack of process around VSCode than it
>> does
>>>>> about Github the tool.
>>>>>
>>>>
>>>> True. I was just pointing out that is not the kind of process I
>> would
>>>> personally want to adopt.
>>>>
>>>
>>> You won't find disagreement here, but this "process" is not due to
>> the
>>> tool. You can just as well allow Thomas to merge stuff without any
>>> review because he has commit rights, no Github needed - and you would
>> be
>>> faced with the same problem.
>>>
>>> So, i don't think Jerin was suggesting that we degrade our
>> merge/commit
>>> rules. Rather, the point was that (whatever you think of VSCode's
>>> review/merge process) there are a lot of pull requests and there is
>>> healthy community collaboration. I'm not saying we don't have that,
>>
>> Yes, recent survey said the process was fine:
>> 	http://mails.dpdk.org/archives/announce/2019-June/000268.html
>>
>>
>>> obviously, but i have a suspicion that we'll get more of it if we
>> lower
>>> the barrier for entry (not the barrier for merge!). I think there is
>> a
>>> way to lower the secondary skill level needed to contribute to DPDK
>>> without lowering coding/merge standards with it.
> 
> That is exactly what I am asking for: Lowering the barrier and increasing the feeling of success for newcomers. (The barrier for merge is probably fine; I'll leave that discussion to the maintainers.)
> 
>>
>> About the barrier for entry, maybe it is not obvious because I don't
>> communicate a lot about it, but please be aware that I (and other
>> maintainers I think) are doing a lot of changes in newcomer patches
>> to avoid asking them knowing the whole process from the beginning.
>> Then frequent contributors get educated on the way.
> 
> Great! I wish that every developer would think and behave this way.

Part of the problem is, there are two different maintainers here: 
maintainers like myself, who maintain a certain area of the code, and 
maintainers like Thomas, who has *commit rights* and maintains the 
entire tree.

And therein lies the problem: Thomas (David, etc.) doesn't look at every 
area of the code, he relies on us to do it. However, *he* is doing the 
committing, and fixing up patches, etc. - so, i can't really say things 
like, "hey, your indentation's wrong here, but Thomas will fix it on 
apply" because that's me pushing more work onto Thomas, something i 
don't think i have the moral right to do :)

So, while Thomas is free to "fix on apply" at his own desire, i don't 
think we have to make this a habit.

> 
>>
>> I think the only real barrier we have is to sign the patch
>> with a real name and send an email to right list.
>> The ask for SoB real name is probably what started this thread
>> in Morten's mind. And the SoB requirement will *never* change.
> 
> The incorrect Signed-off-by might be the only hard barrier (which we cannot avoid). But that did not trigger me.
> 
> I was raising the discussion to bring attention to soft barriers for contributors. What triggered me was the request to split the patch into multiple patches; a kind of feedback I have seen before. For an experienced git user, this is probably very easy, but for a git newbie (like myself), it basically means starting all over and trying to figure out the right set of git commands to do this, which can be perceived as a difficult task requiring a lot of effort.
> 
> Perhaps we could supplement the Contributor Guidelines with a set of cookbooks for different steps in the contribution process, so reviewers can be refer newcomers to the relevant of these as part of the feedback. Just like any professional customer support team has a set of canned answers ready for common customer issues. (Please note: I am not suggesting adding an AI/ML chat bot reviewer to the mailing list!)

That's a great idea, although it's arguably slightly out of scope for 
DPDK. Then again, we do have a "fixline" instructions, so why not have a 
"git reset HEAD^ && git add -p" in there while we're at it :)

> 
> The amount of Contributor Guideline documentation is also a balance... it must be long enough to contain the relevant information to get going, but short enough for newcomers to bother reading it.
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Jerin Jacob]

On Tue, May 26, 2020 at 3:13 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 25-May-20 7:44 PM, Morten Brørup wrote:
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> >> Sent: Monday, May 25, 2020 6:29 PM
> >>
> >> 25/05/2020 18:09, Burakov, Anatoly:
> >>> On 25-May-20 5:04 PM, Maxime Coquelin wrote:
> >>>> On 5/25/20 5:59 PM, Burakov, Anatoly wrote:
> >>>>> On 25-May-20 4:52 PM, Maxime Coquelin wrote:
> >>>>>> On 5/25/20 5:35 PM, Jerin Jacob wrote:
> >>>>>>> On May 25, 2020 Thomas Monjalon wrote:
> >>>>>>>> My concern about clarity is the history of the discussion.
> >>>>>>>> When we post a new versions in GitHub, it's very hard to keep
> >> track
> >>>>>>>> of the history.
> >>>>>>>> As a maintainer, I need to see the history to understand what
> >> happened,
> >>>>>>>> what we are waiting for, and what should be merged.
> >>>>>>>
> >>>>>>> IMO, The complete history is available per pull request URL.
> >>>>>>> I think, Github also email notification mechanism those to
> >> prefer to see
> >>>>>>> comments in the email too.
> >>>>>>>
> >>>>>>> In addition to that, Bugzilla, patchwork, CI stuff all
> >> integrated into
> >>>>>>> one place.
> >>>>>>> I am quite impressed with vscode community collaboration.
> >>>>>>> https://github.com/Microsoft/vscode/pulls
> >>>>>>
> >>>>>> Out of curiosity, just checked the git history and I'm not that
> >>>>>> impressed. For example last commit on the master branch:
> >>>>>>
> >>>>>>
> >> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb74
> >> 46b3915d6148
> >>>>>>
> >>>>>>
> >>>>>> Commit title: " Fix #98530 "
> >>>>>> Commit message empty, no explanation on what the patch is doing.
> >>>>>>
> >>>>>> Then, let's check the the issue it is pointed to:
> >>>>>> https://github.com/microsoft/vscode/issues/98530
> >>>>>>
> >>>>>> Issue is created 15 minutes before the patch is being merged. All
> >> that
> >>>>>> done by the same contributor, without any review.
> >>>>>>
> >>>>>
> >>>>> Just because they do it wrong doesn't mean we can't do it right :)
> >> This
> >>>>> says more about Microsoft's lack of process around VSCode than it
> >> does
> >>>>> about Github the tool.
> >>>>>
> >>>>
> >>>> True. I was just pointing out that is not the kind of process I
> >> would
> >>>> personally want to adopt.
> >>>>
> >>>
> >>> You won't find disagreement here, but this "process" is not due to
> >> the
> >>> tool. You can just as well allow Thomas to merge stuff without any
> >>> review because he has commit rights, no Github needed - and you would
> >> be
> >>> faced with the same problem.
> >>>
> >>> So, i don't think Jerin was suggesting that we degrade our
> >> merge/commit
> >>> rules. Rather, the point was that (whatever you think of VSCode's
> >>> review/merge process) there are a lot of pull requests and there is
> >>> healthy community collaboration. I'm not saying we don't have that,
> >>
> >> Yes, recent survey said the process was fine:
> >>      http://mails.dpdk.org/archives/announce/2019-June/000268.html
> >>
> >>
> >>> obviously, but i have a suspicion that we'll get more of it if we
> >> lower
> >>> the barrier for entry (not the barrier for merge!). I think there is
> >> a
> >>> way to lower the secondary skill level needed to contribute to DPDK
> >>> without lowering coding/merge standards with it.
> >
> > That is exactly what I am asking for: Lowering the barrier and increasing the feeling of success for newcomers. (The barrier for merge is probably fine; I'll leave that discussion to the maintainers.)
> >
> >>
> >> About the barrier for entry, maybe it is not obvious because I don't
> >> communicate a lot about it, but please be aware that I (and other
> >> maintainers I think) are doing a lot of changes in newcomer patches
> >> to avoid asking them knowing the whole process from the beginning.
> >> Then frequent contributors get educated on the way.
> >
> > Great! I wish that every developer would think and behave this way.
>
> Part of the problem is, there are two different maintainers here:
> maintainers like myself, who maintain a certain area of the code, and
> maintainers like Thomas, who has *commit rights* and maintains the
> entire tree.

Yes. I had a similar thought when I said about the "fine-grained"
maintainership/ownership.
The patchwork does not reflect the fine-grained owner of this patch series.
IMO, patch review will speed up or improve if we give the
responsibility of the patch to
specific maintainer based on the MAINTAINERS file.

Github picks up the default owner as CODEOWNERS(The PRIMARY one
responsible for the file or section of the code)
 https://github.com/zephyrproject-rtos/zephyr/blob/master/CODEOWNERS.

The policy for merge permission( Can CODEOWNERS merge the patch) will
be specific to the project.
IMO, This scheme would enable CODEOWNERS are more responsible for the
code review and
we have need system where query what is pending the CODEOWERS.
This http://patches.dpdk.org/project/dpdk/ list does not depict the
CODEOWERS in DPDK.



>
> And therein lies the problem: Thomas (David, etc.) doesn't look at every
> area of the code, he relies on us to do it. However, *he* is doing the
> committing, and fixing up patches, etc. - so, i can't really say things
> like, "hey, your indentation's wrong here, but Thomas will fix it on
> apply" because that's me pushing more work onto Thomas, something i
> don't think i have the moral right to do :)
>
> So, while Thomas is free to "fix on apply" at his own desire, i don't
> think we have to make this a habit.
>
> >
> >>
> >> I think the only real barrier we have is to sign the patch
> >> with a real name and send an email to right list.
> >> The ask for SoB real name is probably what started this thread
> >> in Morten's mind. And the SoB requirement will *never* change.
> >
> > The incorrect Signed-off-by might be the only hard barrier (which we cannot avoid). But that did not trigger me.
> >
> > I was raising the discussion to bring attention to soft barriers for contributors. What triggered me was the request to split the patch into multiple patches; a kind of feedback I have seen before. For an experienced git user, this is probably very easy, but for a git newbie (like myself), it basically means starting all over and trying to figure out the right set of git commands to do this, which can be perceived as a difficult task requiring a lot of effort.
> >
> > Perhaps we could supplement the Contributor Guidelines with a set of cookbooks for different steps in the contribution process, so reviewers can be refer newcomers to the relevant of these as part of the feedback. Just like any professional customer support team has a set of canned answers ready for common customer issues. (Please note: I am not suggesting adding an AI/ML chat bot reviewer to the mailing list!)
>
> That's a great idea, although it's arguably slightly out of scope for
> DPDK. Then again, we do have a "fixline" instructions, so why not have a
> "git reset HEAD^ && git add -p" in there while we're at it :)
>
> >
> > The amount of Contributor Guideline documentation is also a balance... it must be long enough to contain the relevant information to get going, but short enough for newcomers to bother reading it.
> >
>
>
> --
> Thanks,
> Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Thomas Monjalon]

26/05/2020 12:16, Jerin Jacob:
> Burakov, Anatoly wrote:
> > On 25-May-20 7:44 PM, Morten Brørup wrote:
> > > From: Thomas Monjalon
> > >> About the barrier for entry, maybe it is not obvious because I don't
> > >> communicate a lot about it, but please be aware that I (and other
> > >> maintainers I think) are doing a lot of changes in newcomer patches
> > >> to avoid asking them knowing the whole process from the beginning.
> > >> Then frequent contributors get educated on the way.
> > >
> > > Great! I wish that every developer would think and behave this way.
> >
> > Part of the problem is, there are two different maintainers here:
> > maintainers like myself, who maintain a certain area of the code, and
> > maintainers like Thomas, who has *commit rights* and maintains the
> > entire tree.

Let's call these roles "component maintainer" and "tree maintainer".


> Yes. I had a similar thought when I said about the "fine-grained"
> maintainership/ownership.
> The patchwork does not reflect the fine-grained owner of this patch series.

Indeed, patchwork is about patch integration status.
The delegated person is the one responsible for merge, not review.

> IMO, patch review will speed up or improve if we give the
> responsibility of the patch to
> specific maintainer based on the MAINTAINERS file.

I partially disagree about being strict on review responsibility.
Reviews can and should be done by anyone in the community.
This is how we scale: avoid bottlenecks.
Reminder: multiple reviewers are better, and should be the standard.

The component maintainer responsibility is doing some reviews of course,
but the main responsibility is to make sure reviews are done.


> Github picks up the default owner as CODEOWNERS(The PRIMARY one
> responsible for the file or section of the code)
>  https://github.com/zephyrproject-rtos/zephyr/blob/master/CODEOWNERS.

The git-send-email option --cc-cmd devtools/get-maintainer.sh
does a good job at finding code owners.


> The policy for merge permission( Can CODEOWNERS merge the patch) will
> be specific to the project.
> IMO, This scheme would enable CODEOWNERS are more responsible for the
> code review and
> we have need system where query what is pending the CODEOWERS.
> This http://patches.dpdk.org/project/dpdk/ list does not depict the
> CODEOWERS in DPDK.

Not sure I understood your proposal. You would like one more column
in patchwork which is automatically filled with code owners?


> > And therein lies the problem: Thomas (David, etc.) doesn't look at every
> > area of the code, he relies on us to do it. However, *he* is doing the
> > committing, and fixing up patches, etc. - so, i can't really say things
> > like, "hey, your indentation's wrong here, but Thomas will fix it on
> > apply" because that's me pushing more work onto Thomas, something i
> > don't think i have the moral right to do :)

You can send a new version of the patch with the details fixed,
publicly readable, reviewable, and ready to be pushed.


> > So, while Thomas is free to "fix on apply" at his own desire, i don't
> > think we have to make this a habit.

Yes, it should be more or less an exception.

Bottom line, it is important to be transparent and predictable,
while keeping some flexibility.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Burakov, Anatoly]

On 26-May-20 11:33 AM, Thomas Monjalon wrote:

> 
>>> And therein lies the problem: Thomas (David, etc.) doesn't look at every
>>> area of the code, he relies on us to do it. However, *he* is doing the
>>> committing, and fixing up patches, etc. - so, i can't really say things
>>> like, "hey, your indentation's wrong here, but Thomas will fix it on
>>> apply" because that's me pushing more work onto Thomas, something i
>>> don't think i have the moral right to do :)
> 
> You can send a new version of the patch with the details fixed,
> publicly readable, reviewable, and ready to be pushed.

To be completely honest, that's something that's never occurred to me, 
and it sounds like a great idea. The downside is that taking over 
someone else's patch and resubmitting it may be taken the wrong way :) 
(and could also lead to confusion e.g. regarding versioning)

> 
> 
>>> So, while Thomas is free to "fix on apply" at his own desire, i don't
>>> think we have to make this a habit.
> 
> Yes, it should be more or less an exception.
> 
> Bottom line, it is important to be transparent and predictable,
> while keeping some flexibility.
> 
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Thomas Monjalon]

26/05/2020 12:52, Burakov, Anatoly:
> On 26-May-20 11:33 AM, Thomas Monjalon wrote:
> 
> >>> And therein lies the problem: Thomas (David, etc.) doesn't look at every
> >>> area of the code, he relies on us to do it. However, *he* is doing the
> >>> committing, and fixing up patches, etc. - so, i can't really say things
> >>> like, "hey, your indentation's wrong here, but Thomas will fix it on
> >>> apply" because that's me pushing more work onto Thomas, something i
> >>> don't think i have the moral right to do :)
> > 
> > You can send a new version of the patch with the details fixed,
> > publicly readable, reviewable, and ready to be pushed.
> 
> To be completely honest, that's something that's never occurred to me, 
> and it sounds like a great idea. The downside is that taking over 
> someone else's patch and resubmitting it may be taken the wrong way :) 
> (and could also lead to confusion e.g. regarding versioning)

It happens to me to continuing work started by someone else.
I keep original authorship, add my Signed-off-by, increment versioning,
and insert it in the original thread with --in-reply-to.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Burakov, Anatoly]

On 26-May-20 1:45 PM, Thomas Monjalon wrote:
> 26/05/2020 12:52, Burakov, Anatoly:
>> On 26-May-20 11:33 AM, Thomas Monjalon wrote:
>>
>>>>> And therein lies the problem: Thomas (David, etc.) doesn't look at every
>>>>> area of the code, he relies on us to do it. However, *he* is doing the
>>>>> committing, and fixing up patches, etc. - so, i can't really say things
>>>>> like, "hey, your indentation's wrong here, but Thomas will fix it on
>>>>> apply" because that's me pushing more work onto Thomas, something i
>>>>> don't think i have the moral right to do :)
>>>
>>> You can send a new version of the patch with the details fixed,
>>> publicly readable, reviewable, and ready to be pushed.
>>
>> To be completely honest, that's something that's never occurred to me,
>> and it sounds like a great idea. The downside is that taking over
>> someone else's patch and resubmitting it may be taken the wrong way :)
>> (and could also lead to confusion e.g. regarding versioning)
> 
> It happens to me to continuing work started by someone else.
> I keep original authorship, add my Signed-off-by, increment versioning,
> and insert it in the original thread with --in-reply-to.
> 

No, confusion not on your side, but on the side of the person whose 
patch has been taken over :)

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Thomas Monjalon]

26/05/2020 15:57, Burakov, Anatoly:
> On 26-May-20 1:45 PM, Thomas Monjalon wrote:
> > 26/05/2020 12:52, Burakov, Anatoly:
> >> On 26-May-20 11:33 AM, Thomas Monjalon wrote:
> >>
> >>>>> And therein lies the problem: Thomas (David, etc.) doesn't look at every
> >>>>> area of the code, he relies on us to do it. However, *he* is doing the
> >>>>> committing, and fixing up patches, etc. - so, i can't really say things
> >>>>> like, "hey, your indentation's wrong here, but Thomas will fix it on
> >>>>> apply" because that's me pushing more work onto Thomas, something i
> >>>>> don't think i have the moral right to do :)
> >>>
> >>> You can send a new version of the patch with the details fixed,
> >>> publicly readable, reviewable, and ready to be pushed.
> >>
> >> To be completely honest, that's something that's never occurred to me,
> >> and it sounds like a great idea. The downside is that taking over
> >> someone else's patch and resubmitting it may be taken the wrong way :)
> >> (and could also lead to confusion e.g. regarding versioning)
> > 
> > It happens to me to continuing work started by someone else.
> > I keep original authorship, add my Signed-off-by, increment versioning,
> > and insert it in the original thread with --in-reply-to.
> 
> No, confusion not on your side, but on the side of the person whose 
> patch has been taken over :)

I understood it correctly :)
This is what I call collaboration.
The best is to have good communication with its peers,
then workload sharing becomes a detail, in my opinion.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDKcontribution processes

[Jerin Jacob]

On Tue, May 26, 2020 at 4:04 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/05/2020 12:16, Jerin Jacob:
> > Burakov, Anatoly wrote:
> > > On 25-May-20 7:44 PM, Morten Brørup wrote:
> > > > From: Thomas Monjalon
> > > >> About the barrier for entry, maybe it is not obvious because I don't
> > > >> communicate a lot about it, but please be aware that I (and other
> > > >> maintainers I think) are doing a lot of changes in newcomer patches
> > > >> to avoid asking them knowing the whole process from the beginning.
> > > >> Then frequent contributors get educated on the way.
> > > >
> > > > Great! I wish that every developer would think and behave this way.
> > >
> > > Part of the problem is, there are two different maintainers here:
> > > maintainers like myself, who maintain a certain area of the code, and
> > > maintainers like Thomas, who has *commit rights* and maintains the
> > > entire tree.
>
> Let's call these roles "component maintainer" and "tree maintainer".
>
>
> > Yes. I had a similar thought when I said about the "fine-grained"
> > maintainership/ownership.
> > The patchwork does not reflect the fine-grained owner of this patch series.
>
> Indeed, patchwork is about patch integration status.
> The delegated person is the one responsible for merge, not review.

Yes. That's the problem. Merge will happen after the review.
Who is the owner of the review? It should be
"component maintainer" and it needs to track somewhere to see
the status and know the trend for release.
That make more transparent and know the fate on the patch on early
stage.


>
> > IMO, patch review will speed up or improve if we give the
> > responsibility of the patch to
> > specific maintainer based on the MAINTAINERS file.
>
> I partially disagree about being strict on review responsibility.
> Reviews can and should be done by anyone in the community.
> This is how we scale: avoid bottlenecks.
> Reminder: multiple reviewers are better, and should be the standard.

Yes. No disagreement on that. Anyone free to review. None of the above proposed
the scheme is stopping it.

>
> The component maintainer responsibility is doing some reviews of course,
> but the main responsibility is to make sure reviews are done.
>
>
> > Github picks up the default owner as CODEOWNERS(The PRIMARY one
> > responsible for the file or section of the code)
> >  https://github.com/zephyrproject-rtos/zephyr/blob/master/CODEOWNERS.
>
> The git-send-email option --cc-cmd devtools/get-maintainer.sh
> does a good job at finding code owners.
>
>
> > The policy for merge permission( Can CODEOWNERS merge the patch) will
> > be specific to the project.
> > IMO, This scheme would enable CODEOWNERS are more responsible for the
> > code review and
> > we have need system where query what is pending the CODEOWERS.
> > This http://patches.dpdk.org/project/dpdk/ list does not depict the
> > CODEOWERS in DPDK.
>
> Not sure I understood your proposal. You would like one more column
> in patchwork which is automatically filled with code owners?

Yes and the patch's primary _review_ responsibly will be with code owners
and "tree maintainer" apply the patch in fixed time when gets ack from
primary code
owner and if there is no comment from "tree maintainers".


>
>
> > > And therein lies the problem: Thomas (David, etc.) doesn't look at every
> > > area of the code, he relies on us to do it. However, *he* is doing the
> > > committing, and fixing up patches, etc. - so, i can't really say things
> > > like, "hey, your indentation's wrong here, but Thomas will fix it on
> > > apply" because that's me pushing more work onto Thomas, something i
> > > don't think i have the moral right to do :)
>
> You can send a new version of the patch with the details fixed,
> publicly readable, reviewable, and ready to be pushed.
>
>
> > > So, while Thomas is free to "fix on apply" at his own desire, i don't
> > > think we have to make this a habit.
>
> Yes, it should be more or less an exception.
>
> Bottom line, it is important to be transparent and predictable,
> while keeping some flexibility.

I agree.

>
>

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Jerin Jacob]

On Mon, May 25, 2020 at 9:22 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 5/25/20 5:35 PM, Jerin Jacob wrote:
> > On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >>
> >> 25/05/2020 16:28, Burakov, Anatoly:
> >>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
> >>>> 25/05/2020 13:58, Jerin Jacob:
> >>>>> 25/05/2020 11:34, Morten Brørup:
> >>>>>> sending patches over an
> >>>>>> email as opposed to a well-integrated web interface workflow is so alien
> >>>>>> to most people that it definitely does discourage new contributions.
> >>>>>>
> >>>>>> I understand the advantages of mailing lists (vendor independence,
> >>>>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
> >>>>>> for a while (we use those internally), going through DPDK mailing list
> >>>>>> and reviewing code over email fills me with existential dread, as the
> >>>>>> process feels so manual and 19th century to me.
> >>>>>
> >>>>> Agree. I had a difference in opinion when I was not using those tools.
> >>>>> My perspective changed after using Github and Gerrit etc.
> >>>>>
> >>>>> Github pull request and integrated public CI(Travis, Shippable ,
> >>>>> codecov) makes collaboration easy.
> >>>>> Currently, in patchwork, we can not assign a patch other than the set
> >>>>> of maintainers.
> >>>>> I think, it would help the review process if the more fine-grained
> >>>>> owner will be responsible for specific
> >>>>> patch set.
> >>>>
> >>>> The more fine-grain is achieved with Cc in mail.
> >>>> But I understand not everybody knows/wants/can configure correctly
> >>>> an email client. Emails are not easy for everybody, I agree.
> >>>>
> >>>> I use GitHub as well, and I really prefer the clarity of the mail threads.
> >>>> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
> >>>> I think contribution quality would be worst if using GitHub.
> >>>
> >>> I have more experience with Gitlab than Github, but i really don't see
> >>> it that way.
> >>>
> >>> For one, reviewing in Gitlab makes it easier to see context in which
> >>> changes appear. I mean, obviously, you can download the patch, apply it,
> >>> and then do whatever you want with it in your editor/IDE, but it's just
> >>> so much faster to do it right in the browser. Reviewing things with
> >>> proper syntax highlighting and side-by-side diff with an option to see
> >>> more context really makes a huge difference and is that much faster.
> >>
> >> OK
> >>
> >>
> >>> I would also vehemently disagree with the "clarity" argument. There is
> >>> enforced minimum standard of clarity of discussion in a tool such as
> >>> Gitlab. I'm sure you noticed that some people top-post, some
> >>> bottom-post. Some will remove extraneous lines of patches while some
> >>> will leave on comment in a 10K line patch and leave the rest as is, in
> >>> quotes. Some people do weird quoting where they don't actually quote but
> >>> just copy text verbatim, making it hard to determine where the quote
> >>> starts. If the thread is long enough, you'd see the same text quoted
> >>> over and over and over. All of that is not a problem within a single
> >>> patch email, but it adds up to lots of wasted time on all sides.
> >>
> >> Yes
> >>
> >> My concern about clarity is the history of the discussion.
> >> When we post a new versions in GitHub, it's very hard to keep track
> >> of the history.
> >> As a maintainer, I need to see the history to understand what happened,
> >> what we are waiting for, and what should be merged.
> >
> > IMO, The complete history is available per pull request URL.
> > I think, Github also email notification mechanism those to prefer to see
> > comments in the email too.
> >
> > In addition to that, Bugzilla, patchwork, CI stuff all integrated into
> > one place.
> > I am quite impressed with vscode community collaboration.
> > https://github.com/Microsoft/vscode/pulls
>
> Out of curiosity, just checked the git history and I'm not that
> impressed. For example last commit on the master branch:
>
> https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148
>
> Commit title: " Fix #98530 "
> Commit message empty, no explanation on what the patch is doing.

Yes. The merging rules, how much review is required, sanity test to
per check-in will be specific to the project requirements.
I can see zephyr RTOS project(I am following this project) will be
more close to the coding standard and other requirements.

https://github.com/zephyrproject-rtos/zephyr/pulls


>
> Then, let's check the the issue it is pointed to:
> https://github.com/microsoft/vscode/issues/98530
>
> Issue is created 15 minutes before the patch is being merged. All that
> done by the same contributor, without any review.
>

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

On 25-May-20 4:22 PM, Thomas Monjalon wrote:
> 25/05/2020 16:28, Burakov, Anatoly:
>> On 25-May-20 1:53 PM, Thomas Monjalon wrote:
>>> 25/05/2020 13:58, Jerin Jacob:
>>>> 25/05/2020 11:34, Morten Brørup:
>>>>> sending patches over an
>>>>> email as opposed to a well-integrated web interface workflow is so alien
>>>>> to most people that it definitely does discourage new contributions.
>>>>>
>>>>> I understand the advantages of mailing lists (vendor independence,
>>>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>>>> for a while (we use those internally), going through DPDK mailing list
>>>>> and reviewing code over email fills me with existential dread, as the
>>>>> process feels so manual and 19th century to me.
>>>>
>>>> Agree. I had a difference in opinion when I was not using those tools.
>>>> My perspective changed after using Github and Gerrit etc.
>>>>
>>>> Github pull request and integrated public CI(Travis, Shippable ,
>>>> codecov) makes collaboration easy.
>>>> Currently, in patchwork, we can not assign a patch other than the set
>>>> of maintainers.
>>>> I think, it would help the review process if the more fine-grained
>>>> owner will be responsible for specific
>>>> patch set.
>>>
>>> The more fine-grain is achieved with Cc in mail.
>>> But I understand not everybody knows/wants/can configure correctly
>>> an email client. Emails are not easy for everybody, I agree.
>>>
>>> I use GitHub as well, and I really prefer the clarity of the mail threads.
>>> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
>>> I think contribution quality would be worst if using GitHub.
>>
>> I have more experience with Gitlab than Github, but i really don't see
>> it that way.
>>
>> For one, reviewing in Gitlab makes it easier to see context in which
>> changes appear. I mean, obviously, you can download the patch, apply it,
>> and then do whatever you want with it in your editor/IDE, but it's just
>> so much faster to do it right in the browser. Reviewing things with
>> proper syntax highlighting and side-by-side diff with an option to see
>> more context really makes a huge difference and is that much faster.
> 
> OK
> 
> 
>> I would also vehemently disagree with the "clarity" argument. There is
>> enforced minimum standard of clarity of discussion in a tool such as
>> Gitlab. I'm sure you noticed that some people top-post, some
>> bottom-post. Some will remove extraneous lines of patches while some
>> will leave on comment in a 10K line patch and leave the rest as is, in
>> quotes. Some people do weird quoting where they don't actually quote but
>> just copy text verbatim, making it hard to determine where the quote
>> starts. If the thread is long enough, you'd see the same text quoted
>> over and over and over. All of that is not a problem within a single
>> patch email, but it adds up to lots of wasted time on all sides.
> 
> Yes
> 
> My concern about clarity is the history of the discussion.
> When we post a new versions in GitHub, it's very hard to keep track
> of the history.
> As a maintainer, I need to see the history to understand what happened,
> what we are waiting for, and what should be merged.

And AFAIK you do have access to discussion for older versions of the PR, 
do you not? Again, i didn't do in-depth reviews with multiple revisions 
and threads on Github, but assuming Gitlab works similarly, we do have 
access to that.

> 
> 
>> And all of the above will not be a problem with a tool like
>> Gitlab/Github. There are "general" comments that can be used for general
>> discussion, and there are line-specific comments that can be used to
>> discuss certain sections of the patch. I've done this many times in many
>> reviews, and it works very well. Now, granted, I've never maintained an
>> entire repository like DPDK, so you may have a different perspective,
>> but i really don't see how long email chains have "clarity" that a
>> discussion thread with proper quoting, links to code, markdown syntax,
>> etc. doesn't.
> 
> You don't have discussion threading in GitHub. Is there?

Threading is implicit when you are commenting under a line of code. Of 
course this rests on an assumption that you wouldn't use a random 
comment thread to bring up things from another thread, but nothing is 
perfect :)

> 
> 
>> (for the record, i don't consider Gerrit to be a good tool because it
>> enforces a particular git workflow, one that is not at all compatible
>> with how our community works. GitLab, on the other hand, "just works" -
>> i'm assuming GitHub is very similar)
>>
>>>
>>> There is a mailing list discussing workflow tooling:
>>> 	https://lore.kernel.org/workflows/
> 
> 
> 


-- 
Thanks,
Anatoly

Re: [dpdk-dev] Consider improving the DPDK contribution processes

[Wiles, Keith]

> On May 25, 2020, at 7:53 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 25/05/2020 13:58, Jerin Jacob:
>> 25/05/2020 11:34, Morten Brørup:
>>> sending patches over an
>>> email as opposed to a well-integrated web interface workflow is so alien
>>> to most people that it definitely does discourage new contributions.
>>> 
>>> I understand the advantages of mailing lists (vendor independence,
>>> universal compatibility, etc.), but after doing reviews in Github/Gitlab
>>> for a while (we use those internally), going through DPDK mailing list
>>> and reviewing code over email fills me with existential dread, as the
>>> process feels so manual and 19th century to me.
>> 
>> Agree. I had a difference in opinion when I was not using those tools.
>> My perspective changed after using Github and Gerrit etc.
>> 
>> Github pull request and integrated public CI(Travis, Shippable ,
>> codecov) makes collaboration easy.
>> Currently, in patchwork, we can not assign a patch other than the set
>> of maintainers.
>> I think, it would help the review process if the more fine-grained
>> owner will be responsible for specific
>> patch set.
> 
> The more fine-grain is achieved with Cc in mail.
> But I understand not everybody knows/wants/can configure correctly
> an email client. Emails are not easy for everybody, I agree.
> 
> I use GitHub as well, and I really prefer the clarity of the mail threads.
> GitHub reviews tend to be line-focused, messy and not discussion-friendly.
> I think contribution quality would be worst if using GitHub.
> 
> There is a mailing list discussing workflow tooling:
> 	https://lore.kernel.org/workflows/

I disagree about GitHub/GitLab clarity the comments on the patch are inline with the code and these tools provide tracking of these comments. I believed GitHub/GitLab would help others contribute to DPDK easier and faster. With GitHub or GitLab it is a learning curve, but we do not need to have extra tools like patchwork it is already integrated into these tools. Submitting a patch should be simple and not require using N number of tools to submit a patch or manage a patch.

Being able to view the patches inline with code makes reviewing much easier and then being able to rebase the MR with a fixed approval method is great. These tools like any tools has it’s limits, but I think the advantage of these tools out weights some of the comments I have seen for using them.

Beginning able to track pretty much everything in one tool is great as it provides a much easier way to review patches and it focuses on the patch not looking up email messages to make a comment. We only need to comment via the tool at the exact location in the code. These tools also help track these comments and not have to figure out which of the N number of emails I need to respond too. Using the processes of the Linux kernel development is not the most user friendly method of development for developers IMHO.

Adding clang-format to commit-hook is a great way to keep the coding style going. It does not cover every case in DPDK, but they are minor IMHO. This means we can convert to one of these pre-canned formats and then adjust DPDK coding style.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Bruce Richardson]

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

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Burakov, Anatoly]

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 :)

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Jerin Jacob]

On Mon, May 25, 2020 at 8:34 PM Burakov, Anatoly
<anatoly.burakov@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

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Stephen Hemminger]

On Mon, 25 May 2020 13:08:19 +0100
Bruce Richardson <bruce.richardson@intel.com> 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

Or just adjust the coding style to match a clang format.
For a positive example of a project that does this see VPP. They have:
	make checkstyle
and	make fixstyle

And their CI bot checks it.

Re: [dpdk-dev] [dpdk-techboard] Consider improving the DPDK contribution processes

[Bruce Richardson]

On Mon, May 25, 2020 at 08:47:23AM -0700, Stephen Hemminger wrote:
> On Mon, 25 May 2020 13:08:19 +0100
> Bruce Richardson <bruce.richardson@intel.com> 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
> 
> Or just adjust the coding style to match a clang format.
> For a positive example of a project that does this see VPP. They have:
> 	make checkstyle
> and	make fixstyle
> 
> And their CI bot checks it.

Yes, that was what I was implying by asking how big the delta was. :-) If
there are just a couple of things that don't quite align, the benefit of
getting clang-format outweighs the downsides of tweaking our coding
standards, IMHO.