On Thu, Oct 30, 2014 at 1:04 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ronnie Sahlberg <sahlberg@xxxxxxxxxx> writes:
>
>> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
>> index 2a0de42..8f64feb 100644
>> --- a/Documentation/git-send-pack.txt
>> +++ b/Documentation/git-send-pack.txt
>> @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
>> SYNOPSIS
>> --------
>> [verse]
>> -'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
>> +'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]
>>
>> DESCRIPTION
>> -----------
>> @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
>> Send a "thin" pack, which records objects in deltified form based
>> on objects not included in the pack to reduce network traffic.
>>
>> +--atomic-push::
>> + With atomic-push all refs are updated in one single atomic transaction.
>> + This means that if any of the refs fails then the entire push will
>> + fail without changing any refs.
>
> Whenever you say "This means that", please read it twice to see if
> everything before and including that phrase can be removed. It is a
> sign that you found what you wrote before it is not understandable
> and what follows is the version that would be understood by the
> readers.
I reworded it for the next re-roll.
>
>> @@ -203,6 +203,13 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
>> case REF_STATUS_REJECT_NEEDS_FORCE:
>> case REF_STATUS_REJECT_STALE:
>> case REF_STATUS_REJECT_NODELETE:
>> + if (atomic_push_failed && args->use_atomic_push) {
>
> Hmph. Can we even get atomis_push_failed when args->use_atomic_push
> is not set?
It can, but it looks wrong to have one variable passed in via an
argument and the other variable being a global.
I changed it to be
if (atomic_push_failed) {
instead and only pass in a non-NULL atomic_push_failed by the caller
iff args->use_atomic_push is true.
>
>> + fprintf(stderr, "Atomic push is not possible "
>> + "for ref %s. Status:%d\n", ref->name,
>> + ref->status);
>> + *atomic_push_failed = 1;
>> + }
>> + /* fallthrough */
>
> Is this "is not possible" (hence we do not let the user use it), or
> "failed" (i.e. we tried and we failed)?
>
I reworded it to say failed instead.
Will be part of next re-roll.
Thanks
Ronnie Sahlberg
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html