Re: [PATCH 0/4] Expose and manage PCI device reset

From: Amey Narkhede
Date: Fri Mar 12 2021 - 14:08:10 EST


On 21/03/12 07:58PM, Krzysztof Wilczyński wrote:
> Hi Amey,
>
> Thank you for sending the series over!
>
> [...]
> > > Reviews/Acks/Sign-off-by from others (aside from Tested/Reported-by)
> > > really need to be explicit, IMO. This is a common issue for new
> > > developers, but it really needs to be more formal. I wouldn't claim to
> > > be able to speak for Raphael and interpret his comments so far as his
> > > final seal of approval.
> > >
> > > Also in the patches, all Sign-offs/Reviews/Acks need to be above the
> > > triple dash '---' line. Anything between that line and the beginning
> > > of the diff is discarded by tools. People will often use that for
> > > difference between version since it will be discarded on commit.
> > > Likewise, the cover letter is not committed, so Review-by there are
> > > generally not done. I generally make my Sign-off last in the chain and
> > > maintainers will generally add theirs after that. This makes for a
> > > chain where someone can read up from the bottom to see how this commit
> > > entered the kernel. Reviews, Acks, and whatnot will therefore usually
> > > be collected above the author posting the patch.
> > >
> > > Since this is a v1 patch and it's likely there will be more revisions,
> > > rather than send a v2 immediately with corrections, I'd probably just
> > > reply to the cover letter retracting Raphael's Review-by for him to
> > > send his own and noting that you'll fix the commit reviews formatting,
> > > but will wait for a bit for further comments before sending a new
> > > version.
> > >
> > > No big deal, nice work getting it sent out. Thanks,
> > >
> > > Alex
> > >
> > Raphael sent me the email with
> > Reviewed-by: Raphael Norwitz <raphael.norwitz@xxxxxxxxxxx> that
> > is why I included it.
> > So basically in v2 I should reorder tags such that Sign-off will be
> > the last. Did I get that right? Or am I missing something?
> [...]
>
> I am not sure about the messages outside of the mailing list between
> you, Alex and Raphael, as normally conversation and any reviews would
> happen here (on the mailing list, that is), but as long as everyone
> involved is on the same page, then every should be fine.
>
> In terms of how to format the patch, have a look at the following,
> especially before you send another version, as there are some good tips
> and recommendations there (including how to order things):
>
> https://lore.kernel.org/linux-pci/20171026223701.GA25649@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> Krzysztof
Basically whole thing boils down to I'm not good at handling terminal
email clients. I'll surely keep those points mentioned by Bjorn
in my mind.

Thanks,
Amey