All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Dave Olszewski <cxreg@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] builtin/push.c: make push_default a static variable
Date: Tue, 17 Feb 2015 05:46:28 -0500	[thread overview]
Message-ID: <20150217104628.GA25978@peff.net> (raw)
In-Reply-To: <20150216054754.GB25088@peff.net>

On Mon, Feb 16, 2015 at 12:47:54AM -0500, Jeff King wrote:

> When the "push_default" flag was originally added, it was
> made globally visible to all code. This might have been
> useful if other commands or library calls ended up depending
> on it, but as it turns out, only builtin/push.c cares.
> 
> Let's make it a static variable in builtin/push.c.
>
> [...]
> 
> ---
> We know this is safe because no other callers needed tweaked when the
> variable went out of scope. :) It would only be a bad idea if we
> were planning on having other code in the future depend on push_default
> (e.g., the code in remote.c to find the push destination). But it does
> not seem to have needed that in the intervening years, so it's probably
> fine to do this cleanup now.

I had a nagging feeling that there was some code which wanted to use
this elsewhere, and I did finally find it, when I merged this topic with
my other personal topics.

If we wanted to implement "@{push}" (or "@{publish}") to mean "the
tracking ref of the remote ref you would push to if you ran git-push",
then this is a step in the wrong direction.

The patches I posted last January (and which you carried as
jk/branch-at-publish for a while) do work, and I've used the feature
once or twice since then. From the discussion, it looks like they were
meant to be a building block for more triangular-flow work, but I don't
remember what else was needed. I'm tempted to resurrect them, but it's
not a high priority for me.

Anyway, food for thought on whether we want to do this cleanup or not,
then. We can always leave this here as part of git_default_config, and
still move Dave's new option into git_push_config.

-Peff

  parent reply	other threads:[~2015-02-17 10:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-16  3:01 [PATCH] push: allow --follow-tags to be set by config push.followTags Dave Olszewski
2015-02-16  5:20 ` Jeff King
2015-02-16  5:45   ` [PATCH 0/2] clean up push config callbacks Jeff King
2015-02-16  5:46     ` [PATCH 1/2] git_push_config: drop cargo-culted wt_status pointer Jeff King
2015-02-16  5:47     ` [PATCH 2/2] builtin/push.c: make push_default a static variable Jeff King
2015-02-16  5:57       ` Junio C Hamano
2015-02-17 10:46       ` Jeff King [this message]
2015-02-17 17:45         ` Junio C Hamano
2015-02-17 18:23           ` Jeff King
2015-02-17 22:16             ` Junio C Hamano
2015-02-18 18:50               ` Jeff King
2015-02-18 19:08                 ` Junio C Hamano
2015-02-18 19:25                   ` Jeff King
2015-02-18 19:50                     ` Junio C Hamano
2015-02-18 20:03                       ` Jeff King
2015-02-16  5:54     ` [PATCH 3/2] push: allow --follow-tags to be set by config push.followTags Jeff King
2015-02-16  6:02       ` Junio C Hamano
2015-02-16  6:10         ` [PATCH 0/3] cleaner bit-setting in cmd_push Jeff King
2015-02-16  6:12           ` [PATCH 1/3] cmd_push: set "atomic" bit directly Jeff King
2015-02-16  6:13           ` [PATCH 2/3] cmd_push: pass "flags" pointer to config callback Jeff King
2015-02-16  7:05             ` Junio C Hamano
2015-02-16  7:16               ` Jeff King
2015-02-16  6:16           ` [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags Jeff King
2015-03-14  6:06             ` Junio C Hamano
2015-03-14 17:34               ` Jeff King
2015-03-14 17:50               ` Dave Olszewski
2015-03-14 22:08                 ` Junio C Hamano
2015-02-16  6:11         ` [PATCH 3/2] " Junio C Hamano
2015-02-16  6:17           ` Jeff King

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=20150217104628.GA25978@peff.net \
    --to=peff@peff.net \
    --cc=cxreg@pobox.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.