Re: [PATCHv3] Documentation: triangular workflow

[Jordan DE GEA <jordan.de-gea@grenoble-inp.org>]

> Le 7 juin 2016 à 21:12, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> Jordan DE GEA <jordan.de-gea@grenoble-inp.org> writes:
> 
>> +
>> +- Taking the last version of the project by fetching (e.g.
>> +  **UPSTREAM**)
> 
> "by fetching (e.g. UPSTREAM)" does not finish the sentence nicely.
> 
> "... by fetching from **UPSTREAM**" would work better.  So would
> "Fetching the latest version from the project (e.g. UPSTREAM) ».

Ok. Changed for the next iteration. 

>> +- Writing modifications and push them to a fork (e.g. **PUBLISH**)
>> +- Opening a pull request
>> +- Checking of changes by the maintainer and, merging them into the
>> +  **UPSTREAM** repository if accepted
> 
> You'd want to end these sentences with full-stop, by the way
> 

OK too. 

>> +........................................
>> +------------------               -----------------
>> +| UPSTREAM       |  maintainer   | PUBLISH       |
>> +|  git/git       |- - - - - - - -|  me/remote    |
>> +------------------       ←       -----------------
>> +              \                     /
>> +               \                   /
>> +          fetch↓\                 /↑push
>> +                 \               /
>> +                  \             /
>> +                   -------------
>> +                   |   LOCAL   |
>> +                   -------------
>> +........................................
>> +
>> +Git options to use:
>> +~~~~~~~~~~~~~~~~~~~
>> + - `branch.<branch>.remote`
>> + - `branch.<branch>.pushRemote`
>> + - `remote.pushDefault`
>> + - `push.default`
>> +
>> +See linkgit:git-config[1].
> 
> The title says "options" but listed are configuration variables and
> the referred document is also about git-config.  Perhaps retitle it to
> 
> 	Useful configuration variables
>        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 

Ok too. 


> or something like that?
> 
> One thing after reading the above lines that immediately came to my
> mind was this:
> 
>    After listing these four and telling the reader to "See ...", is
>    there anything else the reader needs to learn from below?
> 
> It may make the result a lot more useful document if this gives an
> impression to the reader as if you are saying (you do not have to
> actually say it) "We will guide you how to set up your workflow in
> triangular way, and here are the key configuration variables you
> will end up using; don't worry about the details of them, we'll
> teach you all about them soon in the following paragraphs."
> 
> And I found that "See linkgit:git-config[1]" go directly against
> that line of narrative.

A link is already added at the end of the document. So I delete this one. 

> 
>> +Push behaviour
>> +~~~~~~~~~~~~~~
>> + ...
>> +
>> +Case 2: LOCAL is a clone of **UPSTREAM**
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +'In this case, the remote named `origin` corresponds to **UPSTREAM**.'
>> +
>> +Adding **PUBLISH** remote:
>> +
>> +===================================
>> +* `git remote add publish <PUBLISH_url>`
>> +===================================
> 
> It may perhaps be only me, but these blind instructions puts me off,
> and what I find lacking is "Why should I do this?"  "What benefit do
> I get by doing this".  Start it perhaps like this?
> 
> 	Because you will be pushing into the publish repository
> 	often, instead of having to type its URL every time, you
> 	want a short name you can use to call it.
> 
> and then give that "remote add".

Ok. Similar text added for the next iteration. 

>> +
>> +**Method 1: One option for all branches**
>> +
>> +Setting `remote.pushDefault` in order to push to **PUBLISH** without
>> +argument for push.
>> +
>> +===================================
>> +* `git config remote.pushDefault publish`
>> +===================================
> 
> This is not too bad, but I'd say
> 
> 	With the "remote add" above, you can say "git push publish"
> 	to push there, instead of saying its URL.  But you may want
> 	to be even lazier and say just "git push".  To do so:
> 

Ok too. Similar text added for the next iteration. 

> As a document that is geared toward being a tutorial, I personally
> think it is better to stick to one arrangement rather than
> presenting case 1/2 as two equivalently valid arrangements and
> describe them to equal degree of detail.  Otherwise, after finishing
> reading Case 1 and immediately reading Case 2 heading, the reader
> would start wondering "Which one should I pick?  What are the pros
> and cons?".
> 
> A typical reader of this document would have an upstream in mind,
> perhaps a clone of it locally, and may or may not yet have a publish
> repository, so one valid choice could be to use Case 2.
> 
> Whichever one you choose, the description should not begin with
> "pushing".  A reader who is the target of this document (i.e. who
> owns the LOCAL and PUBLISH repository) begins by cloning and/or
> fetching, followed by working on her own change while staying up to
> date, and pushing is the last thing she does in the flow.
> 
> So I'd recommend reordering the description to
> 
>    * Introduction.  As a summary, here are the four configuration
>      variables you'll be using to make it easier to arrange.
> 
>    * "Preparation".  Clone from the upstream, create an empty
>      publish repository and set it as a secondary remote, with
>      pushdefault pointing at it.
> 
>    * "Staying up-to-date".  You do not have to describe "git fetch"
>      or "git pull" from the upstream aka origin with too much
>      detail, as having or not having a publish repository does not
>      change anything on this side.
> 
>    * "Making your work available".  You would want to reiterate the
>      fact that "git push" does not go to the upstream but to your
>      publishing place thanks to the earlier pushdefault
>      configuration.
> 
>    * "Alternatively...".  In this section, you could mention
>      possible other arrangements.  One could be to set pushdefault
>      for each and every branch (aka your Case 2/Method 2), which
>      shouldn't be necesssary because at the beginning of the
>      document we made it clear that we assume that the reader
>      cannot push to upstream--the normal place she would be pushing
>      is to her own publishing place, and configuring "usually all
>      of them go to my publishing place, but this one alone will go
>      someplace else" (1) is an advanced workflow element, and more
>      importantly (2) is not specific to triangular workflow.
> 
>      Another altenative arrangement worth mentioning may be your
>      Case 1, i.e. to point at your publish place and a secondary
>      "upstream" pointing at where your upstream publishes their
>      work.  You can describe what needs to be changed compared to
>      the above three sections.

Yes, that clearer like that. I reorganize and send a new patch.