Brandon Casey <casey@xxxxxxxxxxxxxxx> 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
