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 <avarab@xxxxxxxxx> 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@xxxxxxxxx
