Re: [RFC][PATCH v3 01/10] fs: common implementation of file type

From: Amir Goldstein
Date: Wed Oct 24 2018 - 05:45:04 EST


On Wed, Oct 24, 2018 at 12:31 PM Phillip Potter <phil@xxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 24, 2018 at 12:20:14PM +0300, Amir Goldstein wrote:
> > On Wed, Oct 24, 2018 at 11:21 AM Phillip Potter <phil@xxxxxxxxxxxxxxxx> wrote:
> > > Dear Amir,
> > >
> > > Yes, I applied each patch manually to my tree, fixed it up where needed,
> > > then after rebuilding and testing each one I committed it and regenerated
> > > each patch. Thank you very much for your advice, I will take it into
> > > account and make the necessary changes. In the meantime, do I add other
> > > tags in the order they are received also (such as Reviewed-by:) and am
> > > I safe to add these in when I re-send the patches with the changes you
> > > and others have suggested (or would that offend people that have
> > > offered the tags)?
> > >
> >
> > Reviewed-by before of after Signed-off-by.
> > I prefer Signed-off-by last which conceptually covers the entire patch,
> > the commit message including all the review tags that you may have added.
> >
> > Some developers add Reviewed-by after Signed-off-by signifying the
> > order that things happened, so choose your own preference.
> >
> > As a reviewer, and I speak only for myself, if I offered my Reviewed-by
> > I expect it to be removed if a future revision of the patch has changed
> > so I have an indication of patches that I need to re-review.
> > But if the patch changed very lightly, like small edits to commit message
> > and code nits in general, that would not invalidate my review.
> > When in doubt, you can always explicitly ask the reviewer.
> >
> > Thanks,
> > Amir.
>
> Dear Amir,
>
> Thanks - I am just going to fix up the commit messages as you suggested
> using git am etc. The content of the patches themselves will not change
> (until further feedback is received).
>

Well, I did request to change some content (the location and the comment
above BUILD_BUG_ON section) which is relevant for several patches.
However, so far affected patched did not get any Reviewed-by.

Thanks,
Amir.