git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jay Soffian" <jaysoffian@gmail.com>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: git@vger.kernel.org, "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches
Date: Tue, 19 Feb 2008 08:40:47 -0500	[thread overview]
Message-ID: <76718490802190540i50cefe15ja8f18e4a397f151d@mail.gmail.com> (raw)
In-Reply-To: <7vprutlchh.fsf@gitster.siamese.dyndns.org>

On Feb 19, 2008 12:52 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Three issues.
>
>  (1) "tells X and Y to configure" is bad.  We may later want to
>      add a command that forks a new branch from an existing one,
>      and you sure do want it to honor this variable.  Better not
>      list the commands but tell the reader that it applies to
>      the act of forking a new branch based on an existing one.
>
>  (2) "configure X and Y" is bad.  We may later want to add new
>      configuration variables to facilitate marking a branch that
>      builds on top of another in addition to the existing remote
>      and merge, and you sure do want them to be also set up
>      appropriately.
>
>  (3) "for use with X and Y" is bad.  We may later want to add
>      new commands that you would use to work with a branch that
>      builds on top of another, and you sure do want them to
>      honor the configuration variables this automatically sets
>      up.
>
> So, how about...
>
>         When creating a new branch starting at an existing 'upstream'
>         branch, the new branch can be marked as building on top of the
>         'upstream' branch, by setting a few configuration variables
>         (e.g. branch.<name>.remote and branch.<name>.merge).  This can
>         be explicitly asked for by giving `--track` (and turned off by
>         giving `--no-track`) option to commands that create a new branch
>         (e.g.  linkgit:git-branch[1]); this variable is consulted when
>         neither option is given.

I disagree. I think explicit is better than implicit and if a new
command or config variable is added, then the docs should be similarly
updated.

> Looks more-or-less unnecessary churn to me.

I was trying to make git-branch(1) and git-checkout(1) consistent.

> > -When a local branch is started off a remote branch, git sets up the
> > -branch so that linkgit:git-pull[1] will appropriately merge from that
> > -remote branch.  If this behavior is not desired, it is possible to
> > -disable it using the global `branch.autosetupmerge` configuration
> > -flag.  That setting can be overridden by using the `--track`
> > -and `--no-track` options.
>
> Why remove this?

Because --track and --no-track are documented in the OPTIONS section, so there
is no reason to document them up top as well. None of the other options [-l],
[-q], [-f] are documented in the intro.

> By the way, I think the existing code is buggy.  When you say:
>
>         $ git branch --track new $(git rev-parse --verify HEAD~12)
>
> the command should barf, saying "Starting point is not a branch; it
> is impossible to set up a tracking information", perhaps without even
> creating the "new" branch.  Instead, it silently creates a new branch.

That's what it did before I started touching any code. Perhaps it
should be a separate cleanup patch.

> >  <start-point>::
> >       The new branch will be created with a HEAD equal to this.  It may
> > -     be given as a branch name, a commit-id, or a tag.  If this option
> > -     is omitted, the current branch is assumed.
> > +     be given as a branch name, a commit-id, or a tag.  Defaults
> > +     to the current HEAD.
>
> I do not think this is what _you_ want (although I do not personally
> care).  Defaulting to HEAD means <start-point> is not a name of the
> branch in such a case.

Okay, that's a subtle difference. The existing documentation conflates (I think)
"HEAD" with "current branch". I see now they are different.

> Now this is getting tiresome.  The rewrite might be a good change for
> readability but this does not have anything to do with --track.  The
> review process of such a rewrite should be done as an independent
> topic.
>
> I give up (for now).

<grumble>
I will rip all the documentation changes out of this patch and do them
separately at a later time if at all. The current documentation has lots of
areas for improvement. It's hard to touch it w/o realizing you've made a clean
spot and then wanting to rewrite whole swaths. I happen to think I'm a decent
technical writer (no, it's not my day job) and I thought the changes I was
making, from the perspective of a new set of eyes, made things more clear. I
give up on that for now.

	
I too find it tiresome trying to get this patch into git. As a newbie
contributor, I'm beginning to find this very discouraging. I appreciate that
changes need a lot of scrutiny, but if the same amount of scrutiny were applied
to the existing code it would never get through.

Which is not to say that I don't appreciate your feedback and work on git.
</grumble>
j.

  reply	other threads:[~2008-02-19 13:41 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-19  2:07 [PATCH] branch: optionally setup branch.*.merge from upstream local branches Jay Soffian
2008-02-19  2:19 ` Jay Soffian
2008-02-19 10:55   ` Johannes Schindelin
2008-02-19 13:42     ` Jay Soffian
2008-02-19 13:59     ` Jay Soffian
2008-02-19 14:01       ` Johannes Schindelin
2008-02-19  5:52 ` Junio C Hamano
2008-02-19 13:40   ` Jay Soffian [this message]
2008-02-19  7:44 ` Alex Riesen
2008-02-19 13:49   ` Jay Soffian
2008-02-19 13:53     ` Johannes Schindelin
2008-02-20  0:13     ` Alex Riesen
2008-02-20  0:48       ` Junio C Hamano
2008-02-20  0:55         ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2008-02-18 13:53 Jay Soffian
2008-02-18 14:05 ` Johannes Schindelin
2008-02-18 14:38   ` Jay Soffian
2008-02-18 18:47 ` Johannes Schindelin
2008-02-18 20:59 ` Junio C Hamano
2008-02-18 13:24 Jay Soffian
2008-02-18 13:29 ` Johannes Schindelin
2008-02-18 19:00   ` Mike Hommey
2008-02-18 12:04 Jay Soffian
2008-02-18 12:14 ` Johannes Schindelin
2008-02-18 12:40   ` Jay Soffian
2008-02-18 13:24     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76718490802190540i50cefe15ja8f18e4a397f151d@mail.gmail.com \
    --to=jaysoffian@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).