Re: [PATCH v2 00/12] submodule: make "git submodule--helper" behave like "git submodule"
From: Glen Choo <hidden>
Date: 2022-06-15 18:43:01
Super happy to see this moving forward, thanks! I'll summarize the discussion to make it easier for others to follow. Ævar Arnfjörð Bjarmason [off-list ref] writes:
This series: * Removes dead code from git-submodule.sh, or dead code where it was the last user.
This is 1-2/12. Not strictly 'necessary', but removing the dead code makes it easier to tell what git-submodule.sh is doing and will make the final review simpler. These all look good to me.
* Simplifies CLI parsing in git-submodule.sh, where it was doing something overly complex for no reason. * Brings "git submodule--helper" in line with the CLI interface of "git-submodule.sh", for a follow-up series to remove the latter, as we'll be able to make a new "git submodule" in C dispatch directly to our new "git submodule--helper" code. * Removes the "-v" option to "git submodule", which has been supported, but was a) never documented b) never did anything anyway, except as a way to negate an earlier "--quiet" option, as "verbose" was always the default.
3-10/12 is the real crux of this series, which removes all of the extra
parsing from "git submodule" by making "git submodule--helper" accept
the same args. This is great, because it means that "submodule--helper"
can just pretend to be "submodule".
One thing that this series _doesn't_ do (but the RFCs do) is to actually
take advantage of this fact and remove the options parsing from
git-submodule.sh, e.g.
cmd_foo() {
git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foo \
${GIT_QUIET:+--quiet} "$@"
}
or in the case dispatch in [1]. Like we discussed earlier, I think this
is for the better - getting rid of the options parsing is (probably)
going to be most error-prone step, so it makes some sense to do that on
its own.
We agreed that 8/12 should be moved to that later series, but everything
else looks good.
* The last couple of patches are cleanup that isn't strictly neccesary for the end-goal of "git submodule" in C, but cleans up some more shellscript code. The "say" function is removed from "git-sh-setup.sh", now that "git-submodule.sh" doesn't use it anymore (which happened before this series) we can replace the couple of remaining uses with "echo", and by having "git-subtree.sh" own the code that used to live in "git-sh-setup.sh".
This is 11-12/12, which removes "say" and related cruft. The changes look ok. I'm not experienced with this area of the codebase, but I've verified the findings in the commit messages. [1] https://lore.kernel.org/git/RFC-patch-09.20-bd0e4a4f8b8-20220610T011725Z-avarab@gmail.com (local)