Re: [PATCH v4] add --summary option to git-push and git-fetch
- From
Larry D'Anna <larry@elder-gods.org>
- Date
- Jan 30, 2010, 00:59 UTC
- Message-ID
- <20100130005948.GA14938@cthulhu>
- In-Reply-To
- <7viqhzm454.fsf@alter.siamese.dyndns.org>
I know it's been a while but.....
Show 12 quoted lines
> > @@ -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?
Show 12 quoted lines
> > + 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.
Show 14 quoted lines
> > + 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.
Show 6 quoted lines
> > -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.
Show 13 quoted lines
> > +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