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.
