how to pre-screen patch submissions from a newbie janitor?

how to pre-screen patch submissions from a newbie janitor?

[Robert P. J. Day]

  not that long ago, i suggested cleaning up the MTD Kconfig/Makefile
structure since there was some definite redundancy in the inclusion of
source files and subdirectories.  now, i could whip up those patches
myself and submit them, but i have a local friend who's itching to get
start doing some simple kernel work and submitting some patches, and
he's more than smart enough to do what i described above.

  however, since he's new to patch creation and submission, i offered
to "vet" his patches first.  i realize that my vetting carries no
weight on the MTD list, but given that i've submitted lots and lots of
patches elsewhere, i can still at least sanity check what he comes up
with before it hits the list.

  is there a canonical signage for that nowadays?  is it "Reviewed-By"
or something like that?  so that by the time it gets to the MTD list,
he would have done the "Signed-Off-By" and i'd sign off as a reviewer.
is that the way it works?  or is there an alternative?  thanks.

rday

-- 

========================================================================
Robert P. J. Day                               Waterloo, Ontario, CANADA

            Linux Consulting, Training and Kernel Pedantry.

Web page:                                          http://crashcourse.ca
Twitter:                                       http://twitter.com/rpjday
========================================================================

Re: how to pre-screen patch submissions from a newbie janitor?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

>   however, since he's new to patch creation and submission, i offered
> to "vet" his patches first.  i realize that my vetting carries no
> weight on the MTD list, but given that i've submitted lots and lots of
> patches elsewhere, i can still at least sanity check what he comes up
> with before it hits the list.
> 
>   is there a canonical signage for that nowadays?  is it "Reviewed-By"
> or something like that?  so that by the time it gets to the MTD list,
> he would have done the "Signed-Off-By" and i'd sign off as a reviewer.
> is that the way it works?  or is there an alternative?  thanks.

Yes, that sounds like a good case for Reviewed-by. It will surely help these
patches knowing there is somebody with experience who already had a look at
(and in that case probably build-tested) them.

Looking forward to the series,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: how to pre-screen patch submissions from a newbie janitor?

[Robert P. J. Day]

On Wed, 19 May 2010, Wolfram Sang wrote:

> >   however, since he's new to patch creation and submission, i
> > offered to "vet" his patches first.  i realize that my vetting
> > carries no weight on the MTD list, but given that i've submitted
> > lots and lots of patches elsewhere, i can still at least sanity
> > check what he comes up with before it hits the list.
> >
> >   is there a canonical signage for that nowadays?  is it
> > "Reviewed-By" or something like that?  so that by the time it gets
> > to the MTD list, he would have done the "Signed-Off-By" and i'd
> > sign off as a reviewer. is that the way it works?  or is there an
> > alternative?  thanks.
>
> Yes, that sounds like a good case for Reviewed-by. It will surely
> help these patches knowing there is somebody with experience who
> already had a look at (and in that case probably build-tested) them.
>
> Looking forward to the series,

  what i'm going to have this trainee(?) do is just simplify the
config/Kconfig/Makefile combos, which i openly admit is not
earth-shaking in its complexity.

  for example. in a number of places elsewhere in the kernel tree,
i've submitted patches to change things like this in the config menus:

<M>   OneNAND Device Support  --->
      LPDDR flash memory drivers  --->
      UBI - Unsorted block images  --->

  note how, to select either LPDDR or UBI, you need to *go* to those
submenus, where you find a top-level selector.  it's simpler to just
put that selector right on that menu item, as it's done for "OneNAND"
just above it.  no wasting time.

  and another simplification is to cut down the conditional inclusion
in the makefiles.  for instance, here's tests/Makefile:

obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
... etc etc ...

  there's little point having every single line in the Makefile test
CONFIG_MTD_TESTS when that can all be done with a single test one
level up that checks whether or not to process the tests/ directory at
all.

  in short, nothing spectacular, just tidying up.

rday

-- 

========================================================================
Robert P. J. Day                               Waterloo, Ontario, CANADA

            Linux Consulting, Training and Kernel Pedantry.

Web page:                                          http://crashcourse.ca
Twitter:                                       http://twitter.com/rpjday
========================================================================

Re: how to pre-screen patch submissions from a newbie janitor?

[Artem Bityutskiy]

On Tue, 2010-05-18 at 19:40 -0400, Robert P. J. Day wrote:
> not that long ago, i suggested cleaning up the MTD Kconfig/Makefile
> structure since there was some definite redundancy in the inclusion of
> source files and subdirectories.  now, i could whip up those patches
> myself and submit them, but i have a local friend who's itching to get
> start doing some simple kernel work and submitting some patches, and
> he's more than smart enough to do what i described above.
> 
>   however, since he's new to patch creation and submission, i offered
> to "vet" his patches first.  i realize that my vetting carries no
> weight on the MTD list, but given that i've submitted lots and lots of
> patches elsewhere, i can still at least sanity check what he comes up
> with before it hits the list.
> 
>   is there a canonical signage for that nowadays?  is it "Reviewed-By"
> or something like that?  so that by the time it gets to the MTD list,
> he would have done the "Signed-Off-By" and i'd sign off as a reviewer.
> is that the way it works?  or is there an alternative?  thanks.

Hi,

MTD list is not something special, common Linux kernel patch submission
practices apply. So obviously, he signs off his patches, you can review,
test, ack or sign off it. I am not pedantic about which tag to use, but
you can always refer to 'Documentation/SubmittingPatches' and pick the
right one.
 
-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: how to pre-screen patch submissions from a newbie janitor?

[Artem Bityutskiy]

On Wed, 2010-05-19 at 07:40 -0400, Robert P. J. Day wrote:
> On Wed, 19 May 2010, Wolfram Sang wrote:
> 
> > >   however, since he's new to patch creation and submission, i
> > > offered to "vet" his patches first.  i realize that my vetting
> > > carries no weight on the MTD list, but given that i've submitted
> > > lots and lots of patches elsewhere, i can still at least sanity
> > > check what he comes up with before it hits the list.
> > >
> > >   is there a canonical signage for that nowadays?  is it
> > > "Reviewed-By" or something like that?  so that by the time it gets
> > > to the MTD list, he would have done the "Signed-Off-By" and i'd
> > > sign off as a reviewer. is that the way it works?  or is there an
> > > alternative?  thanks.
> >
> > Yes, that sounds like a good case for Reviewed-by. It will surely
> > help these patches knowing there is somebody with experience who
> > already had a look at (and in that case probably build-tested) them.
> >
> > Looking forward to the series,
> 
>   what i'm going to have this trainee(?) do is just simplify the
> config/Kconfig/Makefile combos, which i openly admit is not
> earth-shaking in its complexity.
> 
>   for example. in a number of places elsewhere in the kernel tree,
> i've submitted patches to change things like this in the config menus:
> 
> <M>   OneNAND Device Support  --->
>       LPDDR flash memory drivers  --->
>       UBI - Unsorted block images  --->
> 
>   note how, to select either LPDDR or UBI, you need to *go* to those
> submenus, where you find a top-level selector.  it's simpler to just
> put that selector right on that menu item, as it's done for "OneNAND"
> just above it.  no wasting time.

Yeah, and I assume it is better to be consistent and use the same style
as most of things in the MTD menu have.

>   and another simplification is to cut down the conditional inclusion
> in the makefiles.  for instance, here's tests/Makefile:
> 
> obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
> ... etc etc ...
> 
>   there's little point having every single line in the Makefile test
> CONFIG_MTD_TESTS when that can all be done with a single test one
> level up that checks whether or not to process the tests/ directory at
> all.

Yeah, sounds good.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)