[Intel-wired-lan] [net-next] Documentation: Update Intel wired LAN docs
Jeff Kirsher
jeffrey.t.kirsher at intel.com
Thu Feb 8 18:52:46 UTC 2018
On Thu, 2018-02-08 at 10:42 -0800, Shannon Nelson wrote:
> On 2/8/2018 9:57 AM, Jeff Kirsher wrote:
> > On Wed, 2018-02-07 at 16:37 -0800, Shannon Nelson wrote:
> > > On 2/6/2018 1:00 PM, Jeff Kirsher wrote:
> > > > Updated the kernel documentation on e1000e, fm10k, i40e/vf,
> > > > igb/vf
> > > > and
> > > > ixgbe/vf.
> > > >
> > > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher at intel.com>
> > >
> > > I didn't really read this for details, but I saw a few things
> > > that
> > > jumped out at me and noted them below. In general, it would be
> > > nice
> > > to
> > > have someone go through them and do proper updates to make these
> > > files
> > > look consistent so they look like they came from the same
> > > company.
> > > I
> > > know there are differences in the individual drivers, but there
> > > are
> > > a
> > > lot of similarities and these files should not all look so
> > > different.
> > > There's also still a lot of out-dated information here that
> > > doesn't
> > > do
> > > your customer any good.
> > >
> > > Also, with so many things changed in each file, it would be much
> > > easier
> > > to review if each was a separate patch rather than having to page
> > > through a few thousand lines in a single email. You'd be much
> > > more
> > > likely to have someone who knows about a particular driver review
> > > at
> > > least that one file.
> >
> > True, we do need to go through the copyright headers for all the
> > drivers and make sure they are consistent. Not sure if I want to
> > "add"
> > those kind of changes to this patch, but I will take it under
> > consideration.
> >
> > As far as the "one" patch versus many patches, as it stand with
> > just
> > the SPDX change, I am sure David Miller would rather have one patch
> > to
> > make this change versus 8 patches. This is based on my experiences
> > in
> > the past when dealing with changes like this.
> >
>
> I would argue that this is a very different kind of patch than the
> SPDX
> change patches. A single patch with many simple one-line changes is
> often a fairly repetitive and mechanical change, and it is easy
> enough
> to look at the change pattern and the list of files and agree that it
> is
> an acceptable patch.
>
> This patch is changing significant chunks of text in each file in
> several different ways, it is not a simple one line change to each
> file.
> I can't imaging that making these kinds of changes to so many code
> files all in one shot would be accepted.
>
> As the submitting-patches.rst says "The point to remember is that
> each
> patch should make an easily understood change that can be verified
> by
> reviewers." This patch should be split up by the individual driver
> related files in order for reviewers to give good comments on the
> driver
> they know. I sure couldn't give any kind of signoff for this patch
> as I
> know very little about several of the drivers represented.
>
> ... and if the file is so big that I can't get my comments through
> the
> mail listserver, what good is asking for a review?
So sorry, I had the SPDX patch in mind when I was reading your response
and so I responded thinking the SPDX patch was what you were commenting
on. You are correct, I should break this patch up into each driver
documentational change.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20180208/2ebcbbb6/attachment.asc>
More information about the Intel-wired-lan
mailing list