[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