Re: [PATCH] revisions: refactor init_revisions and setup_revisions.

[Junio C Hamano <gitster@pobox.com>]

Pierre Habouzit <madcoder@debian.org> writes:

>   We could of course have a .def member in the struct rev_info, and use
> the one passed to setup_revisions then if it's still NULL, but it
> doesn't really makes sense to me, and I don't really see a problem with
> saying at init time that you'll default to "HEAD". Though if you really
> dislike it that much, I squash a patch that does that on top of it.

Well, it was not liking or disliking.  Although I thought "default" that
sets a value to the default after the parser finds that the user did not
give anything (the approach you described in the above quoted paragraph)
is a natural implementation, probably more so than what you did, I do not
have strong preference either way.

When I see a change where I do not see a reason to, I get suspicious,
wondering if I am missing some bigger reason (e.g. "by moving it there
this and that would become much easier and cleaner, even though it now
forces callers of cmd_log_init() to duplicate the default values").  There
must be an obvious justification you had when you changed it, which I am
not seeing.  Hence that question.

>> Applying this to 'master' and then merging 'pu' shows that there are a few
>> topics that are cooking that would conflict with this change.  Merging
>> 'next' seems to go cleanly (I haven't checked the result), so it is not
>> too bad for me to carrry this at this moment, if we were not this close to
>> the rc freeze.  I dunno.
>
>   Well I can wait longer, I'd just like to see it merged in a not too
> far future, because I have to check for new places that would need
> conversions at each reabase :)

Yeah, that burden can be shifted to me, in other words ;-)