git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Chris Rorvick <chris@rorvick.com>
Subject: Re: [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE
Date: Thu, 24 Jan 2013 01:43:26 -0500	[thread overview]
Message-ID: <20130124064326.GB610@sigill.intra.peff.net> (raw)
In-Reply-To: <7vip6nj22m.fsf@alter.siamese.dyndns.org>

On Wed, Jan 23, 2013 at 08:28:49AM -0800, Junio C Hamano wrote:

> How about doing this?
> 
> For "needs force" cases, we say this instead:
> 
>  hint: you cannot update a ref that points at a non-commit object, or
>  hint: update a ref to point at a non-commit object, without --force.
> 
> Being explicit about "non-commit" twice will catch user's eyes and
> cause him to double check that it is not a mistyped LHS of the push
> refspec (if he is sending a non-commit) or mistyped RHS (if the ref
> is pointing at a non-commit).  If he _is_ trying to push a blob out,
> the advice makes it clear what to do next: he does want to force it.

Yeah, I think that is sensible.

> Note that you _could_ split the "needs force" case into two, namely,
> "cannot replace a non-commit" and "cannot push a non-commit".  You
> could even further split them [...etc...]

I do not think it is worth worrying too much about. This should really
not happen very often, and the user should be able to investigate and
figure out what is going on. I think making the error message extremely
specific is just going to end up making it harder to understand.

> If we did that, then we could loosen the "You should fetch first"
> case to say something like this:
> 
>  hint: you do not have the object at the tip of the remote ref;
>  hint: perhaps you want to pull from there first?

Yeah, better. I'll comment on the specific message you used in response
to the patch itself.

> This explicitly denies one of Chris's wish "we shouldn't suggest to
> merge something that we may not be able to", but in the "You should
> fetch first" case, we cannot fundamentally know if we can merge
> until we fetch.  I agree with you that the most common case is that
> the unknown object is a commit, and that suggesting to pull is a
> good compromise.

I thought the wish was more about "we shouldn't suggest to merge
something we _know_ we will not be able to", and you are still handling
that (i.e., the "needs force" case).

-Peff

  reply	other threads:[~2013-01-24  6:43 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-30  1:41 [PATCH v6 0/8] push: update remote tags only with force Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 1/8] push: return reject reasons as a bitset Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 2/8] push: add advice for rejected tag reference Chris Rorvick
2012-12-02 10:42   ` Junio C Hamano
2012-12-03  3:27     ` [PATCH 0/2] push: honor advice.* configuration Chris Rorvick
2012-12-03  3:27       ` [PATCH 1/2] push: rename config variable for more general use Chris Rorvick
2012-12-03  3:27       ` [PATCH 2/2] push: allow already-exists advice to be disabled Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 3/8] push: flag updates Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 4/8] push: flag updates that require force Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 5/8] push: require force for refs under refs/tags/ Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 6/8] push: require force for annotated tags Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 7/8] push: clarify rejection of update to non-commit-ish Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 8/8] push: cleanup push rules comment Chris Rorvick
2012-12-02 20:43   ` [PATCH] remote.c: fix grammatical error in comment Chris Rorvick
2012-12-03 18:53 ` [PATCH v6 0/8] push: update remote tags only with force Junio C Hamano
2013-01-16 13:32 ` Max Horn
2013-01-16 16:00   ` Junio C Hamano
2013-01-16 16:01   ` Jeff King
2013-01-16 17:10     ` Junio C Hamano
2013-01-16 17:43       ` Jeff King
2013-01-16 21:02         ` Junio C Hamano
2013-01-17  2:19         ` Chris Rorvick
2013-01-17  3:11           ` Jeff King
2013-01-17  3:42             ` Chris Rorvick
2013-01-16 16:36   ` Junio C Hamano
2013-01-16 16:48     ` Junio C Hamano
2013-01-17  6:20       ` Chris Rorvick
2013-01-17  6:59         ` Junio C Hamano
2013-01-17 13:09           ` Chris Rorvick
2013-01-18  1:06             ` Jeff King
2013-01-18  3:18               ` Chris Rorvick
2013-01-21 23:40                 ` Jeff King
2013-01-21 23:53                   ` Junio C Hamano
2013-01-22  4:59                   ` Chris Rorvick
2013-01-22  6:44                     ` Junio C Hamano
2013-01-22  5:53                   ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-22  5:53                     ` [PATCH 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-22  5:53                     ` [PATCH 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-22  6:04                       ` Junio C Hamano
2013-01-22  5:53                     ` [PATCH 3/3] push: further reduce "struct ref" and simplify the logic Junio C Hamano
2013-01-22  6:21                       ` Junio C Hamano
2013-01-22  6:30                   ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-22  6:30                     ` [PATCH v2 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-23  6:43                       ` Jeff King
2013-01-22  6:30                     ` [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-23  6:56                       ` Jeff King
2013-01-23 16:28                         ` Junio C Hamano
2013-01-24  6:43                           ` Jeff King [this message]
2013-01-22  6:30                     ` [PATCH v2 3/3] push: further simplify the logic to assign rejection status Junio C Hamano
2013-01-22  7:26                     ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-23 21:55                   ` [PATCH v4 " Junio C Hamano
2013-01-23 21:55                     ` [PATCH v4 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-24 22:22                       ` Eric Sunshine
2013-01-23 21:55                     ` [PATCH v4 2/3] push: further simplify the logic to assign rejection reason Junio C Hamano
2013-01-23 21:55                     ` [PATCH v4 3/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-24  6:58                       ` Jeff King
2013-01-24 17:19                         ` Junio C Hamano
2013-01-25  4:31                     ` [PATCH v4 0/3] Finishing touches to "push" advises Chris Rorvick
2013-01-25  5:04                       ` Junio C Hamano
2013-01-25  5:14                         ` Chris Rorvick
2013-01-18  4:36               ` [PATCH v6 0/8] push: update remote tags only with force Junio C Hamano

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=20130124064326.GB610@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=chris@rorvick.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 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).