Re: [PATCH v4] add --summary option to git-push and git-fetch

[Larry D'Anna <larry@elder-gods.org>]

I know it's been a while but.....

> > @@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
> >  				fputc(url[i], fp);
> >  		fputc('\n', fp);
> >  
> > -		if (ref)
> > -			rc |= update_local_ref(ref, what, note);
> > -		else
> > +		if (ref) {
> > +			*quickref = 0;
> > +			rc |= update_local_ref(ref, what, note, quickref);
> 
> Makes me wonder why update_local_ref() does not put that NUL upon entry.

I'm not sure what you mean.  Could you elaborate?

> > +	init_revisions(&rev, NULL);
> > +	rev.prune = 0;
> > +	assert(!handle_revision_arg(quickref, &rev, 0, 1));
> > +	assert(!prepare_revision_walk(&rev));
> > +
> > +	while ((commit = get_revision(&rev)) != NULL) {
> > +		struct strbuf buf = STRBUF_INIT;
> > +		if (limit == 0) {
> > +			fprintf(stderr, "    ...\n");
> 
> How would you know, when you asked 20 and you showed 20 here, that there
> is no more to come?

If there's more it will print the "...", if there isn't then it won't.

> > +			break;
> > +		}
> 
> > +		if (!commit->buffer) {
> > +			enum object_type type;
> > +			unsigned long size;
> > +			commit->buffer =
> > +				read_sha1_file(commit->object.sha1, &type, &size);
> > +			if (!commit->buffer)
> > +				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
> > +		}
> > +		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
> 
> Hmm, why so many spaces before %m and after %m?

So the summary lines are nicely indented with respect to the other output.

> > -static int do_push(const char *repo, int flags)
> > +static int do_push(const char *repo, int flags, int summary)
> 
> Couldn't this be just another bit in the flag?  I didn't check but I
> suspect you wouldn't have to touch the intermediate functions in the call
> chain that way.

It can't just be a bit because the "summary" parameter contains number of
summary lines to print.

> > +test_expect_success 'fetch --summary forced update' '
> > +	mk_empty &&
> > +	(
> > ...
> > +	)
> > +
> > +'
> 
> There are at least two missing combinations. (1) "fetch --summary" to
> fetch a new branch, and (2) "fetch --summary" does not try segfaulting by
> accessing unavailable information after a failed fetch.
> 
> The same comment applies to the push side of the tests.

What would be a good way to induce a failed fetch for this test?


     --larry