[PATCH] user: Document patch review process

Sebastian Huber sebastian.huber at embedded-brains.de
Tue Oct 1 08:10:06 UTC 2019

On 01/10/2019 09:22, Chris Johns wrote:
> On 1/10/19 3:50 pm, Sebastian Huber wrote:
>> On 01/10/2019 01:40, Chris Johns wrote:
>>> On 30/9/19 10:45 pm, Sebastian Huber wrote:
>>>> +* The patch builds.  All RTEMS tests link with this patch.
>>>> +
>>>> +* The patch does not introduce new compiler warnings.
>>> This step is not in the figure.
>> You mean there should be a step mentioning this checklist?
> Sorry, I missed by a line, I meant the patch builds. I am wondering if building
> should be in the figure, that is the figure and this text match. I think it is
> important to point out in a bold manner patches need to build and need to work.
> I know this is common sense but ....

What about this figure:




:Arrange your changes in\nan easy to review and\ncoherent patch series;

:Apply the checklist for patches;

:Invoke: ""git format-patch"";

:Send the patch series to devel at rtems.org for review;

:Set N to 2;

while (Reviewers demand changes in the patch series?) is (Yes)
   :Do the required changes and create a new patch series;

   :Update the commit messages accordingly;

   :Apply the checklist for patches;

   :Invoke: ""git format-patch -v $N"";

   :Document the changes from version N to N + 1\nin the patch file 
after the "---" line;

   :Set N to N + 1;

   :Send the patch series to devel at rtems.org for review;
endwhile (No)

if (Patch series was accepted by reviewers?) then (Yes)
   :Push the patch series\nto the project repository;

   note right
     Must be done by an
     RTEMS maintainer.
   end note
else (No)
   :Discard the patch series;



>>> Built against which BSPs?
>> This is for the user manual. I think at least one arbitrary BSP which is
>> affected by the patch should be sufficient.
> Joel and I now have the BSP builder building on a regular basis because we found
> things broke in weird ways across archs and bsps. If we had a patch smoke test
> tool available it would help but we do not.

Yes, this is the long term goal.

>>>> +* The patch does not introduce new test failures in existing tests.
>>> This step is not in the figure.
>>> Again which BSPs? This assumes expected fails are valid for the bsps being
>>> tested.
>> I think we should not add to many details to the figure.
> The list of steps here does not matching the figure is a potential source of
> user problems if the user inspects the figure and skims the written text.
>>> What about tickets and the "Closes ...", "Updates ..." etc tags?
>> Yes, this is missing. What should be checked as well?
> What does "checked" mean?

I mean items (individual checks) for the Checklist for Patches.

Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

More information about the devel mailing list