[Intel-wired-lan] [net-next] Documentation: Update Intel wired LAN docs

Shannon Nelson shannon.nelson at oracle.com
Thu Feb 8 18:42:02 UTC 2018


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?

sln


More information about the Intel-wired-lan mailing list