From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [net-next] Documentation: Update Intel wired LAN docs
Date: Thu, 08 Feb 2018 10:52:46 -0800 [thread overview]
Message-ID: <1518115966.26676.13.camel@intel.com> (raw)
In-Reply-To: <fae843ac-19e4-53a0-bd18-39e67ba9b2c5@oracle.com>
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@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>
prev parent reply other threads:[~2018-02-08 18:52 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-06 21:00 [Intel-wired-lan] [net-next] Documentation: Update Intel wired LAN docs Jeff Kirsher
2018-02-07 23:04 ` Brown, Aaron F
2018-02-08 0:37 ` Shannon Nelson
2018-02-08 0:41 ` Shannon Nelson
2018-02-08 17:57 ` Jeff Kirsher
2018-02-08 18:42 ` Shannon Nelson
2018-02-08 18:52 ` Jeff Kirsher [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1518115966.26676.13.camel@intel.com \
--to=jeffrey.t.kirsher@intel.com \
--cc=intel-wired-lan@osuosl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.