RE: [PATCH 1/2] Documentation about triangular workflow

From: ALBERTIN TIMOTHEE p1514771 <timothee.albertin@etu.univ-lyon1.fr>
Date: Nov 22, 2017, 15:13 UTC
Message-ID: <1511367199220.88867@etu.univ-lyon1.fr>
In-Reply-To: <CAN0heSoLbnkfJLqStvR2nfy4LAAc3KEkRH-_kE-QAQoM-4iqGA@mail.gmail.com>

>On 17 November 2017 at 17:07, Daniel Bensoussan
><danielbensoussanbohm@gmail.com> wrote:
>> +- If the maintainer accepts the changes, he merges them into the
>> +  **UPSTREAM** repository.

>Personally, I would prefer "they" and "their" over "he" and "his". I'm
>not sure how universal this preference is, but see also 715a51bcaf (am:
>counteract gender bias, 2016-07-08). I realize that "he" is already used
>in this document...

This could be a good thing in order to be neutral. We can change this in the whole file.

Show 5 quoted lines

>> + ... The contributor
>> +was forced to create a mail which shows the difference between the
>> +new and the old code, and then send it to a maintainer to commit
>> +and push it.  This isn't convenient at all, neither for the
>> +contributor, neither for the maintainer.

>"neither ... nor". That said, I find the tone of this paragraph a bit
>value-loaded ("forced ... isn't convenient at all"). It does sort of
>contradict or at least compare interestingly with how git.git itself is
>maintained. ;-) Maybe this could be framed in a more neutral way?

Junio C Hamano told us the same thing about the motivation section, we'll change it the next patch.

>> +The goal of the triangular workflow is also that the rest of the
>> +community or the company can review the code before it's in production.
>> +Everyone can read on **PUBLISH** so everyone can review code
>> +before the maintainer(s) merge it to **UPSTREAM**.  It also means

>I think you can drop the "(s)". Your example workflow could have a
>single maintainer. In a multi-maintainer workflow, the workflow would
>still be the same. So no need to cover all bases by sprinkling "(s)" on
>the text. (IMHO.)

We'll fix that.

Thank you for your review.

Timothée Albertin