On Tue, Jun 4, 2013 at 3:14 PM, Junio C Hamano <gits...@pobox.com> wrote:
> elliottcable <m...@ell.io> writes:
>> Thus, I've added an --authorship-order version of --date-order, which relies
>> upon the AUTHOR_DATE instead of the COMMITTER_DATE; this means that old 
>> commits
>> will continue to show up chronologically in-order despite rebasing.
>> ---
> Missing sign-off.  Please see Documentation/SubmittingPatches.


I read that part, and was rather confused. At no point did I get the
idea that I should sign-off *my own initial commit*. Perhaps that part
of the documentation needs to be slightly re-written? Would that be a
welcome change?

On Tue, Jun 4, 2013 at 3:14 PM, Junio C Hamano <gits...@pobox.com> wrote:
> elliottcable <m...@ell.io> writes:
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 9e21232..54d4d7f 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -237,7 +237,7 @@ static void log_show_early(struct rev_info *revs, struct 
>> commit_list *list)
>>       int i = revs->early_output;
>>       int show_header = 1;
>> -     sort_in_topological_order(&list, revs->lifo);
>> +     sort_in_topological_order(&list, revs->lifo, revs->use_author);
> The name "use-author" is a clear sign that the person who added this
> code were too narrowly focused to think "author" automatically would
> mean "author date" ;-).
> It probably makes sense to revamp sort_in_topological_order(), so
> that its second parameter is not a boolean 'lifo' that tells too
> much about its implementation without telling what it actually
> means.  Instead, we can make it an enum sort_order, that tells it to
> emit the commits in committer-date order, author-date order, or
> graph-traversal order.
> And update revs->lifo to use that same enum, without adding
> use_author_date bit to rev_info.

I'll look into replacing lifo with an enum as soon as I can sit back
down to update this patch. For the moment, nothing more than
committer_date_sort and author_date_sort, I suppose?

Overview being, I suppose, that `lifo` will no longer exist (since it
effectively determines, when truthy, that we operate in a
*non*-date-ordered topological method); then have commiter_date_order
and author_date_order bits in an enum, with zero being
lifo/straightforward-topological-order. Sound about right?

I'll try and make this a separate patch. First commit, to replace lifo
with an enum; second commit, to *actually implement* the code obeying
that enum when it is set to author_date_order.

On Tue, Jun 4, 2013 at 5:22 PM, Jeff King <p...@peff.net> wrote:
> On Tue, Jun 04, 2013 at 12:14:21PM -0700, Junio C Hamano wrote:
>> > diff --git a/commit.h b/commit.h
>> > index 67bd509..de07525 100644
>> > --- a/commit.h
>> > +++ b/commit.h
>> > @@ -17,6 +17,7 @@ struct commit {
>> >     void *util;
>> >     unsigned int indegree;
>> >     unsigned long date;
>> > +   unsigned long author_date;
>> While walking we keep many of them in-core, and 8-byte each for each
>> commit objects add up.  We do not want to make "struct commit" any
>> larger than it already is.
>> Having said that, I do not see a reasonable alternative
>> implementation than adding an author-date field to struct commit
>> without major restructuring if we were to add this feature.
>> So please do not take this part of the response as a "patch rejected
>> because we do not want to add anything to this structure".  We'll
>> think of something down the road, but as an independent topic after
>> this gets in (or doesn't).
> Yeah, I had the same thought. Maybe this is a good candidate to build on
> top of the jk/commit-info slab experiment. The topo-sort could allocate
> an extra slab for author-date (or even expand the indegree slab to hold
> both indegree and author date), use it during the sort, and then free it
> afterwards.
> Elliott: you can see the relevant changes to the topo-sort in commit
> 96c4f4a (commit: allow associating auxiliary info on-demand,
> 2013-04-09).
> -Peff

Again, might be a little over my head. If you really think it's best
that I look into that branch, I will try. :)

Meantime, is there any other, more-immediate approach you can think
of? I thought, for a moment, of only storing *either* the committer
*or* the author date in the commit-struct at a given time, and
flagging with a single bit ... but I'm not sure how widely-spread the
nead for committer-date currently is. Maybe I can go back and
parse-out the author date *when I need it*, instead, though that
sounds slow …

Epilogue: I'll make the *obvious* changes mentioned above sometime
within the next week (I'm more than a little swamped; in the middle of
a big breakup, and a big move, simultaneously! :( ), especially the
enum instead of the use_author bit … and submit another patch RFC. It
won't be finalized until we can decide what to do about the extra
8bytes in the commit struct, though. More input welcome.
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

Reply via email to