Re: [PATCHv3] Documentation: triangular workflow

[Junio C Hamano <gitster@pobox.com>]

Jordan DE GEA <jordan.de-gea@grenoble-inp.org> writes:

> diff --git a/Documentation/gitworkflows.txt b/Documentation/gitworkflows.txt
> index f16c414..3b5fd09 100644
> --- a/Documentation/gitworkflows.txt
> +++ b/Documentation/gitworkflows.txt
> @@ -463,6 +463,156 @@ if you get conflicts: `git am -3` will use index information contained
>  in patches to figure out the merge base.  See linkgit:git-am[1] for
>  other options.
>  
> +TRIANGULAR WORKFLOW
> +-------------------
> +
> +In some projects, you cannot push directly to the project but have to
> +suggest your commits to the maintainer (e.g. pull requests).
> +For these projects, it's common to use what's called a *triangular
> +workflow*:
> +
> +- 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)".

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

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

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.

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

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

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.

so that a first-time reader can learn _one_ workable organization
quickly without having to choose blindly between many choices.

Thanks.