Re: [PATCH 2/5] Make mktag a builtin.

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

Brandon Casey <casey@nrlssc.navy.mil> writes:

> I didn't think I should move the non-builtin mktag.c to builtin-mktag.c,
> and then after I modified mktag to be a builtin I knew I was moving it
> to builtin-tag.c so I didn't see a point to renaming it.

I see.  I did not realize that the eventual shape would be to have both
mktag and tag to be in builtin-tag.c, just like builtin-log.c supports
many other commands from the log family, and that was where my question
came from.

But I think the arrangement to have both in builtin-tag.c actually makes
sense --- mktag needs to be kept supported but most of its internal should
be shared with tag anyway.

> Also, I decided about those things _before_ I realized how small the changes
> would be to mktag to make it a builtin.
>
> Do you think the modified patch you posted conflicts with the idea that
> "code move should be separate from code change"?

Yes, it does, but I do not subscribe to the "idea".  Therefor I do not see
any problem.

If you were to stop at making mktag a builtin, then the patch I sent would
be the change that is necessary to do so.  A code movement can and often
does need some adjustment (e.g. if you move "a.c" to "src/a.c", its
'#include "a.h"' may need to become '#include "../a.h"' (or preferably to
'#include <a.h>' with appropriate -I.. option in the Makefile).  It does
not help anybody to insist on a blanket dogma that forbids modification
and movement at the same time.

We do discourage rolling unrelated things in one commit, but creating a
builtin "foo" typically involves creation of builtin-foo.c and associated
changes to the Makefile and builtin.h, and in this case the initial
contents of builtin-mktag.c happens to come from an existing file mktag.c,
while making the original mktag.c obsolete and unnecessary along the way.
That is a single logical change, and I do not think there is anything
wrong to do it in one commit. In fact, splitting such a change into more
than one commit is just plain silly and wrong, isn't it?