| Message ID | 20210423194230.1388945-1-lukeshu@lukeshu.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | subtree: clean up, improve UX | expand |
On Fri, 23 Apr 2021 13:42:00 -0600, Luke Shumaker wrote: > I promise that > there's more work coming on top of it (we've discovered lots of ways > to break the "subtree split" algorithm, and come up with fixes for > most of them). Follow-up question: If in that work I changed the shebang from "#!/bin/sh" to "#!/usr/bin/env bash" and started using Bash arrays, would that be so bad? Would that be land-able?
On Fri, Apr 23 2021, Luke Shumaker wrote: > On Fri, 23 Apr 2021 13:42:00 -0600, > Luke Shumaker wrote: >> I promise that >> there's more work coming on top of it (we've discovered lots of ways >> to break the "subtree split" algorithm, and come up with fixes for >> most of them). > > Follow-up question: If in that work I changed the shebang from > "#!/bin/sh" to "#!/usr/bin/env bash" and started using Bash arrays, > would that be so bad? Would that be land-able? It's in contrib, so personally I don't care much. But e.g. on OSX that means targeting some ancient (and going away?) version of bash + having *BSDs needing to install bash for this etc. Bash or not it seems to me like that argument parsing code could be much simplified by simply declaring strings of arguments accepted for each sub-command, and then parsing those arguments + dying on unknown ones.
Luke Shumaker <lukeshu@lukeshu.com> writes: > On Fri, 23 Apr 2021 13:42:00 -0600, > Luke Shumaker wrote: >> I promise that >> there's more work coming on top of it (we've discovered lots of ways >> to break the "subtree split" algorithm, and come up with fixes for >> most of them). > > Follow-up question: If in that work I changed the shebang from > "#!/bin/sh" to "#!/usr/bin/env bash" and started using Bash arrays, > would that be so bad? Would that be land-able? I'd rather see "git subtree" tool taken out of my tree and flourish as a standalone project of its own. Over its long history, from time to time people stepped in only to scratch their own itch and then went away. Without having continued presense of an area expert (or two) who can give consistent guidance to the tool's evolution, I feel that Git project itself failed to give sufficient service to users of "git subtree". As I won't be that area expert, and we do not seem to be growing such an area expert who can be responsible for the tool in the long haul, it probably is a disservice to its users to keep it in my tree and pretend that it is maintained to the same degree as the rest of Git. If those who are interested and/or have stake in the "git subtree" tool can unite and take its development in their hands, with their own style, that might be better for the health of the "git subtree" tool in the long run. Thanks.
From: Luke Shumaker <lukeshu@datawire.io> Ostensibly, this patch set is about improving various aspects of `git subtree`'s user interface (and it is!), but it's also mostly about setting the foundation and being "batch 1" of a bunch more changes to subtree that I'm getting queued up. So please forgive the large amount of churn in the leading clean-up commits, I promise that there's more work coming on top of it (we've discovered lots of ways to break the "subtree split" algorithm, and come up with fixes for most of them). In the mean-time, I do think that the UX improvements in this patchset are already worth it themselves. - The first 11 commits improve subtree's tests, largely around the code-quality of the tests, but a few of the commits do actually improve what's being tested. - The middle 12 commits improve the code-quality of subtree's implementation. - The final 7 commits improve various aspects of subtree's user experience, from readability of the debug output, to documentation, to option flag handling. The very last commit is likely to be a little objectionable--it makes some option flag parsing more strict, so there will probably be worry that the change breaks existing users. However, it's being strict about arg combinations that were always invalid, the difference is that now it reports that to the users and bails. Those users were already broken, they just didn't know it. `git subtree` should tell them. As a final question, would it be all right to amend CI to run the subtree tests? And if so, what would be the best way to do it? For my own testing, I just made the following edit to the main Makefile, but I'm not sure it's the mos appropriate approach: --- a/Makefile +++ b/Makefile @@ -2836,6 +2836,7 @@ test: all $(MAKE) -C t/ all + $(MAKE) -C contrib/subtree/ test perf: all $(MAKE) -C t/perf/ all The first two commits are about getting that to pass. I'd prefer to avoid that type of bitrot in the future. Luke Shumaker (30): .gitignore: Ignore /git-subtree subtree: t7900: update for having the default branch name be 'main' subtree: t7900: use test-lib.sh's test_count subtree: t7900: use consistent formatting subtree: t7900: comment subtree_test_create_repo subtree: t7900: use 'test' for string equality subtree: t7900: delete some dead code subtree: t7900: fix 'verify one file change per commit' subtree: t7900: rename last_commit_message to last_commit_subject subtree: t7900: add a test for the -h flag subtree: t7900: add porcelain tests for 'pull' and 'push' subtree: don't have loose code outside of a function subtree: more consistent error propagation subtree: drop support for git < 1.7 subtree: use `git merge-base --is-ancestor` subtree: use git-sh-setup's `say` subtree: use more explicit variable names for cmdline args subtree: use $* instead of $@ as appropriate subtree: give `$(git --exec-path)` precedence over `$PATH` subtree: use "^{commit}" instead of "^0" subtree: parse revs in individual cmd_ functions subtree: remove duplicate check subtree: add comments and sanity checks subtree: don't let debug and progress output clash subtree: have $indent actually affect indentation subtree: give the docs a once-over subtree: allow --squash to be used with --rejoin subtree: allow 'split' flags to be passed to 'push' subtree: push: allow specifying a local rev other than HEAD subtree: be stricter about validating flags .gitignore | 1 + contrib/subtree/git-subtree.sh | 613 +++++++----- contrib/subtree/git-subtree.txt | 184 ++-- contrib/subtree/t/t7900-subtree.sh | 1421 ++++++++++++++++++---------- 4 files changed, 1363 insertions(+), 856 deletions(-)