Hi,
On Sat, 16 Jan 2010 11:23:47 +0200
Ilari Liusvaara <ilari.liusvaara@xxxxxxxxxxx> wrote:
> [snip]
> @@ -218,6 +218,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
> OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
> OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
> OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
> + OPT_BIT('u', "set-upstream", &flags, "Set upstream for git pull", TRANSPORT_PUSH_SET_UPSTREAM),
This line exceeds 80 characters.
Also, I think you should follow the case-style of the other option
descriptions and s/Set(?= upstream)/set/.
> [snip]
> +static void set_upstreams(struct transport *trans, struct ref *refs,
I would prefer if you could follow the naming convention and say
"transport" instead of truncating to "trans".
> + int pretend)
> +{
> + struct ref *i;
> + for (i = refs; i; i = i->next) {
In most places, "ref" instead of "i" is used; ie.
struct ref *ref;
for (ref = refs; ref; ref = ref->next) {
..
}
> [snip]
> + /*
> + * Check suitability for tracking. Must be successful /
> + * alreay up-to-date ref create/modify (not delete).
> + */
s/alreay/already/.
> [snip]
> + /* Chase symbolic refs (mainly for HEAD). */
Did you mean "Change" here?
Regarding the checking of ref->status here:
> @@ -135,6 +136,53 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
> }
> }
>
> [snip]
> + if (i->status != REF_STATUS_OK &&
> + i->status != REF_STATUS_UPTODATE)
> + continue;
Is it possible to delegate this to push_had_errors(remote_refs)
instead? We skip setting up upstream tracking when there are errors
from pushing, so we don't have to check ref->status anymore.
(Note: this would skip setting up upstream when ref->status is 'none',
in addition to 'ok' and 'uptodate', as you have it right now. I think
this should be safe.)
@@ -1002,8 +1055,9 @@ int transport_push(struct transport *transport,
verbose | porcelain, porcelain,
nonfastforward);
- if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
- set_upstreams(transport, remote_refs, pretend);
+ if (!push_had_errors(remote_refs) &&
+ (flags & TRANSPORT_PUSH_SET_UPSTREAM))
+ set_upstreams(transport, remote_refs, pretend);
if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
struct ref *ref;
for (ref = remote_refs; ref; ref = ref->next)
(As a side note, if you apply this on top of 'tr/http-push-ref-status'
in 'pu', the result of push_had_errors() is saved in a variable
('err'), so you could just use it and not have to call push_had_errors
() again.)
--
Cheers,
Ray Chuan
--
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