Re: [RFC PATCH 0/4] docs/devel suggestions for discussion

[Mark Cave-Ayland]

On 12/10/2022 13:11, Alex Bennée wrote:

> Hi,
>
> This is an attempt to improve our processes documentation by:
>
>   - adding an explicit section on maintainers
>   - reducing the up-front verbiage in patch submission
>   - emphasising the importance to respectful reviews
>
> I'm sure the language could be improved further so I humbly submit
> this RFC for discussion.
>
> Alex Bennée (4):
>    docs/devel: add a maintainers section to development process
>    docs/devel: make language a little less code centric
>    docs/devel: simplify the minimal checklist
>    docs/devel: try and improve the language around patch review
>
>   docs/devel/code-of-conduct.rst           |   2 +
>   docs/devel/index-process.rst             |   1 +
>   docs/devel/maintainers.rst               |  84 +++++++++++++++++++
>   docs/devel/submitting-a-patch.rst        | 101 +++++++++++++++--------
>   docs/devel/submitting-a-pull-request.rst |  12 +--
>   roms/qboot                               |   2 +-
>   6 files changed, 157 insertions(+), 45 deletions(-)
>   create mode 100644 docs/devel/maintainers.rst

Hi Alex,

I've replied with a couple of things I noticed, but in general I think this is a 
really nice improvement.

If you're looking at documenting some of the maintainer processes better, there are a 
few other things I have been thinking about that it may be worth discussing:


i) Requiring an R-B tag for all patches before merge

- Is this something we should insist on and document?

ii) Unresponsive maintainers

- Should we have a process for this? When Blue Swirl (the previous SPARC maintainer) 
disappeared abruptly, I think it took nearly 3 months to get my first patches merged 
since no-one knew if they were still active. If a maintainer has been unresponsive 
for e.g. 2 months should that then allow a process where other maintainers can merge 
patches on their behalf and/or start a process of maintainer transition?

iii) Differences(?) between maintainers

- There have been a few instances where I have been delayed in finding time for patch 
review, and in the meantime someone has stepped up to review the patch and given it 
an R-B tag which is great. However I have then reviewed the patch and noticed 
something amiss, and so it needs a bit more work before being merged. I think in 
these cases the review of the maintainer of the code in question should take priority 
over other maintainer reviews: do we need to explicitly document this? I can 
certainly see how this can be confusing to newcomers having an R-B tag as a 
pre-requisite for merge coming from one person, and then a NACK from someone else later.


ATB,

Mark.