Re: [LTP] [PATCH v2] Add goals of patch review and tips

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> >> +The following are examples and may not be appropriate for all tests.
>> >> +
>> >> +* Merge the patch. It should apply cleanly to master.
>> 
>> As a newbie with LTP I am still struggling to understand some things
>> like this one. How is possible to merge to master in order to review?
>
> Obviously you do that to your local git tree. This is basics of git
> development nothing specific to LTP here.

I suppose it would not hurt to add "to your local copy of the master
branch". It's only a few more words that would clean it up (IMO). If
that doesn't make sense then you will have to do a search or ask someone
because this document can't be too long.

>
>> >> +## How to get patches merged
>> 
>> Again from my POV the description is more about what you should do as a
>> reviewer than how to get a patch merged.
>
> Isn't that the same? If you know what are developers doing in order to
> catch common mistakes you can as well avoid doing them...
>

Perhaps we are not doing a good job of marketing patch review in this
document, but it is probably also outside the scope of this document.

>> >> +Once you think a patch is good enough you should add your Reviewed-by
>> >> +and/or Tested-by tags. This means you will get some credit for getting
>> >> +the patch merged. Also some blame if there are problems.
>> >> +
>> >> +If you ran the test you can add the Tested-by tag. If you read the
>> >> +code or used static analysis tools on it, you can add the Reviewed-by
>> >> +tag.
>> >> +
>> >> +In addition you can expect others to review your patches and add their
>> >> +tags. This will speed up the process of getting your patches merged.
>> >> +
>> >> +## Maintainers Checklist
>> >
>> > Looks very nice, thanks for writing this out.
>> >
>> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
>> >
>> > -- 
>> > Cyril Hrubis
>> > chrubis@suse.cz
>> 
>> I feel that this is more an overview and reminder of already
>> contributors. Not sure how helpful is it for new comers like myself
>
> I think that there are different levels of newcommers. I do not think
> that the documment is supposed to help newcommers that are already
> familiar with how git based development works and only highlights
> things that are specific to LTP.

Yup, there is a long tutorial which explains in depth a lot of stuff and
this could be expanded, but I don't have time for that right now.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp