Thread (90 messages) 90 messages, 3 authors, 2022-06-28

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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help