Re: [PATCH v4 0/3] Finishing touches to "push" advises
On Thu, Jan 24, 2013 at 11:04 PM, Junio C Hamano wrote:
> Would it be sufficient to do this? I think "the tag already exists
> in the remote" is already clear that we are talking about the
> destination.
Good point.
> diff --git a/builtin/push.c b/builtin/push.c
> index a2b3fbe..78789be 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -228,7 +228,7 @@ static const char message_advice_ref_fetch_first[] =
>"See the 'Note about fast-forwards' in 'git push --help' for
> details.");
>
> static const char message_advice_ref_already_exists[] =
> - N_("Updates were rejected because the destination reference already
> exists\n"
> + N_("Updates were rejected because the tag already exists\n"
>"in the remote.");
>
> static const char message_advice_ref_needs_force[] =
Looks like the new-line is now unnecessary, but that looks good to me.
Thanks,
Chris
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] Finishing touches to "push" advises
Chris Rorvick writes:
> Had I written the the "already exists" advice in the context of these
> additional statuses I would have said "the destination *tag* reference
> already exists", or maybe even just "the destination *tag* already
> exists".
Yeah, now we do not use "already exists" for anything other than
refs/tags/, right? Your rewording sounds like the right thing to
make it even clearer.
Thanks for bringing it up.
Would it be sufficient to do this? I think "the tag already exists
in the remote" is already clear that we are talking about the
destination.
diff --git a/builtin/push.c b/builtin/push.c
index a2b3fbe..78789be 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -228,7 +228,7 @@ static const char message_advice_ref_fetch_first[] =
"See the 'Note about fast-forwards' in 'git push --help' for
details.");
static const char message_advice_ref_already_exists[] =
- N_("Updates were rejected because the destination reference already
exists\n"
+ N_("Updates were rejected because the tag already exists\n"
"in the remote.");
static const char message_advice_ref_needs_force[] =
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] Finishing touches to "push" advises
On Wed, Jan 23, 2013 at 3:55 PM, Junio C Hamano wrote: > This builds on Chris Rorvick's earlier effort to forbid unforced > updates to refs/tags/ hierarchy and giving sensible error and advise > messages for that case (we are not rejecting such a push due to fast > forwardness, and suggesting to fetch and integrate before pushing > again does not make sense). FWIW, these changes look good to me. The logic in set_ref_status_for_push() is easier to follow and the additional error statuses (and associated advice) make things much clearer. Had I written the the "already exists" advice in the context of these additional statuses I would have said "the destination *tag* reference already exists", or maybe even just "the destination *tag* already exists". It's probably fine the way it is, but I only avoided using "tag" in the advice because I was abusing it. Thanks, Chris -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/3] Finishing touches to "push" advises
This builds on Chris Rorvick's earlier effort to forbid unforced updates to refs/tags/ hierarchy and giving sensible error and advise messages for that case (we are not rejecting such a push due to fast forwardness, and suggesting to fetch and integrate before pushing again does not make sense). The series applies on top of 256b9d7 (push: fix "refs/tags/ hierarchy cannot be updated without --force", 2013-01-16). This fourth round swaps the order of clean-up patches and now the bottom two are clean-up patches. The main change is in the third one. When the object at the tip of the remote is not a committish, or the object we are pushing is not a committish, the existing code already rejects such a push on the client end, but we used the same error and advice messages as the ones used when rejecting a push that does not fast-forward, i.e. pull from there and integrate before pushing again. Introduce a new rejection reason NEEDS_FORCE and explain why the push was rejected, stressing the fact that --force is required when non committish objects are involved, so that the user can (1) notice a possibly mistyped source object name or destination ref name, when the user is trying to push an ordinary commit, or (2) learn that "--force" is an appropriate thing to use when the user is sure that s/he wants to push a non-committish (which is unusual). Unlike the third round, we do not say "fetch first, inspect the situation to decide what to do", when we do not have the object sitting at the tip of the remote. Most likely, it is a commit somebody who has been working on the same branch pushed that we haven't fetched yet, so suggesting to pull is often sufficient and appropriate, and in a more uncommon case in which the unknown object is not a committish, the suggested pull will fail without making permanent damage anywhere. Next atttempt to push without changing anything (e.g. "reset --hard") will then trigger the NEEDS_FORCE "Your push involves non-commit objects" case. Junio C Hamano (3): push: further clean up fields of "struct ref" push: further simplify the logic to assign rejection reason push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Documentation/config.txt | 12 +++- advice.c | 4 advice.h | 2 ++ builtin/push.c | 29 + builtin/send-pack.c | 10 ++ cache.h | 6 +++--- remote.c | 42 +++--- send-pack.c | 2 ++ transport-helper.c | 10 ++ transport.c | 14 +- transport.h | 2 ++ 11 files changed, 105 insertions(+), 28 deletions(-) -- 1.8.1.1.517.g0318d2b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
