Re: rdma kernel tree

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

On Tue, 2015-05-19 at 21:47 +0300, Or Gerlitz wrote:
> On Tue, May 19, 2015 at 3:22 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Tue, 2015-05-19 at 07:35 +0300, Or Gerlitz wrote:
> >> >> I pulled that via a bundle from patchworks.  I'll double check it.
> >> > Did  you check it out? fixed it out?
> >>
> >> I took a look now, you've rebased to-be-rebased/for-4.2 to 4.1-rc4 and
> >
> > Correct.
> >
> >> it seems this is what you are going to push into the kernel.org treem
> >
> > Correct.
> >
> >> but this series is still there with the zillion tested/reviewed/etc
> >> signature per one 2-3 patch, I think we've agreed this needs to be
> >> addressed prior to the upstream push, right?
> >
> > Incorrect.  What you objected to before was the large Cc: list in the
> > patches.  That is gone.  What is there now is just the reviewed-by: list
> > of three people, and the tested-by list of two people.  As the entire
> > patch set as a whole was reviewed and tested by those people, it seems
> > accurate to me.
> 
> Doug, I have never ever seen a patch set (specifically the 15~23 part
> of it) with that level of simplicity

That portion of the patchset didn't start out with that level of
simplicity per patch, it evolved to that because it made review
*significantly* easier.  It's very simple to review a patch that does:

Add 1 helper
Replace tests in code with just that 1 helper

because you can scroll through that patch and know that every line being
replaced is related to that one helper.  If you want to know every line
that was replaced with rdma_cap_iw_cm, you go to that one patch and it's
all listed very easy to read.

On the other hand, when you squash all those patches together, review
becomes much harder because if you want to see what a single helper
does, you have to sift through all of the other helper changes and hope
you find the right ones, and that you found all of the right ones.

>  and
> signature/reviewers/tested-by/etc inflation.

I added those myself as part of an automated addition.  It applied to
the entire series, so it was put on each patch.  The people that
tested/reviewed the series did not do so to individual patches, they hit
all of them.  And as was pointed out a couple of weeks ago on an earlier
patchset I picked up, it is generally good behavior to give attribution
where it is due to encourage people to participate.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]