[PATCH] user: Document patch review process

[Chris Johns]

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:
>>> ---
>>>   images/user/patch-review.png  | Bin 0 -> 57130 bytes
>>>   images/user/patch-review.puml |  44 +++++++++++++++++++++++++++
>>>   user/support/contrib.rst      |  67 +++++++++++++++++++++++++++++++++---------
>>>   3 files changed, 97 insertions(+), 14 deletions(-)
>>>   create mode 100644 images/user/patch-review.png
>>>   create mode 100644 images/user/patch-review.puml
>>>
>>
>> If you want to review the figure it is ...
>>
>> http://www.plantuml.com/plantuml/png/dP0_ZzDC4CRx_HGZwoqIlSf9K8Q28451ST7f3WgaD7lsutZ1EsjcnuxoxLcFWt9790gAbNR-_6QUUNPPlUWOU-VivzpsWuZd8-YSHg6wc_-P0X_OCy7dCsaYmHHm8i_DWUlKGS1AWzUwemm9oE_AeCyyfH_OKbKTWrAR97hTM5DLpVKdS4FQuHKuJsymeT-98kQx9CSPlMnSCANztFQsHASkzA3LerKXkR2ng7gX-sR3-pM5JAjlo6j7jFsOh4FmSmo2AsbJ15v1dXYdFyyhwDUXcSjrYZ4eHUJiZQph94tWOt-slhyOGPk9_jkR7IQb7YDOJT1l7QsaI1CaXyJBtNlwdzuS-DLfxMo3RnLYoMgpsLJK1uPDDi-khEN-pVx2N2pVfxLpeQNLmqlyvEr-38g6diyN3b9SdtVnrVU7CNStwm-iQKbA-evQIJ2a73J9OYKd1KauTbe2elinAps3ciIOjtcszEENJ_TF57rWBGzoLxBWncY7FY_gpV6GQo-tDjYXeNKkQngSsvLeZFql
>>
> 
> Nice, you can use it for new images here:
> 
> http://www.plantuml.com/plantuml/uml/SyfFKj2rKt3CoKnELR1Io4ZDoSa70000
> 

:)

> [...]
>>> +* 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 ....

>>
>> 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.

>>
>>> +* 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?

Chris