| Message ID | 20210423194230.1388945-5-lukeshu@lukeshu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | subtree: clean up, improve UX | expand |
On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote: > The formatting in t7900-subtree.sh isn't even consistent throughout the > file. Fix that; make it consistent throughout the file. > > Signed-off-by: Luke Shumaker <lukeshu@datawire.io> > --- > diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh > @@ -23,26 +21,24 @@ subtree_test_create_repo() > -check_equal() > -{ > +check_equal () { > test_debug 'echo' > test_debug "echo \"check a:\" \"{$1}\"" > test_debug "echo \" b:\" \"{$2}\"" > - if [ "$1" = "$2" ]; then > + if [ "$1" = "$2" ] > + then We prefer `test` over `[`, so it might make sense to update that, as well, along with these other style cleanups.
On Fri, 23 Apr 2021 15:51:53 -0600, Eric Sunshine wrote: > > On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote: > > The formatting in t7900-subtree.sh isn't even consistent throughout the > > file. Fix that; make it consistent throughout the file. > > > > Signed-off-by: Luke Shumaker <lukeshu@datawire.io> > > --- > > diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh > > @@ -23,26 +21,24 @@ subtree_test_create_repo() > > -check_equal() > > -{ > > +check_equal () { > > test_debug 'echo' > > test_debug "echo \"check a:\" \"{$1}\"" > > test_debug "echo \" b:\" \"{$2}\"" > > - if [ "$1" = "$2" ]; then > > + if [ "$1" = "$2" ] > > + then > > We prefer `test` over `[`, so it might make sense to update that, as > well, along with these other style cleanups. OK, I'll include that in this commit.
Eric Sunshine <sunshine@sunshineco.com> writes: >> +check_equal () { >> test_debug 'echo' >> test_debug "echo \"check a:\" \"{$1}\"" >> test_debug "echo \" b:\" \"{$2}\"" >> - if [ "$1" = "$2" ]; then >> + if [ "$1" = "$2" ] >> + then > > We prefer `test` over `[`, so it might make sense to update that, as > well, along with these other style cleanups. If I were working on this, I wouldn't bother. As far as I am concerned, contrib/subtree has always been treated as a borrowed code [*] that is written in a dialect of shell that is different from what our scripts are written in, and there are too many style differences (I wouldn't call them violations---nobody has expected the code there to follow our style, or attempted to enforce our style there) to bother coercing. If Luke is volunteering to take over its maintainership, it would be appreciated by its users. It has been in the "abandonware" status for too long. [Footnote] * ... as opposed to a properly maintained part of the git-core proper.
On Tue, 27 Apr 2021 01:17:38 -0600, Junio C Hamano wrote: > > Eric Sunshine <sunshine@sunshineco.com> writes: > > >> +check_equal () { > >> test_debug 'echo' > >> test_debug "echo \"check a:\" \"{$1}\"" > >> test_debug "echo \" b:\" \"{$2}\"" > >> - if [ "$1" = "$2" ]; then > >> + if [ "$1" = "$2" ] > >> + then > > > > We prefer `test` over `[`, so it might make sense to update that, as > > well, along with these other style cleanups. > > If I were working on this, I wouldn't bother. In this case, it's not just about consistency with git-core, it's about consistency within contrib/subtree; there were just 2 or 3 places where it used `[` instead of `test`. > If Luke is volunteering to take over its maintainership, it would be > appreciated by its users. It has been in the "abandonware" status > for too long. I think I am volunteering. We have been using git-subtree increasingly heavily at Ambassador Labs (née Datawire) for about 2 years now, and I don't see that changing. What I'm doing now is trying to get >2 years of accumulated patches in to a submittable state (a lot of the patches were bad; for instance on my main branch `git subtree add` is broken, but that's fine, because you don't use `add` all the time like you do `split` or `merge`, but that's something I need to fix before submitting it). Assuming that we're going to continue being heavy users of it, and are going to continue to patch issues with it, I'd rather let that live upstream rather than telling all of my coworkers to get it from <https://github.com/LukeShu/git>. With a recent change in project scheduling, I anticipate that I'll have bandwidth to be able to handle that. (It's what's giving me adequate time to work through this pile of existing patches, anyway.) What does being a maintainer consist of? Are there standups that I should join? > As far as I am concerned, contrib/subtree has always been treated as > a borrowed code [*] that is written in a dialect of shell that is > different from what our scripts are written in, and there are too > many style differences (I wouldn't call them violations---nobody has > expected the code there to follow our style, or attempted to enforce > our style there) to bother coercing. > > [Footnote] > > * ... as opposed to a properly maintained part of the git-core > proper. Elsewhere in the thread, you suggested that subtree be taken out of git.git, and live as a standalone project. I'm not entirely opposed to that, but 1. I'm not sure how whoever picks it up (me) establishes their git-subtree as the "real" subtree (get a blessing from Avery?). 2. I think a lot of the reason why more people don't use git-subtree is that the core 'split' operation doesn't quite work reliably (and also it can be quite slow), and so it doesn't get recommended. I would like nothing more than to improve the 'split' reliability to where it does start to gain adoption, to where we can think about it graduating from contrib/ to git-core. 3. Many systems (Arch Linux and macOS, at least) give users git-subtree as part of the stock Git install. If I'm interested in growing git-subtree adoption, I'd be a fool to give that up :) On the other hand, I think that in the long-ish term git-subtree wants to be rewritten in a better-suited language. My personal inclination would be Go, but if I ever want it to graduate to git-core, it'd have to be C, huh?
Luke Shumaker <lukeshu@lukeshu.com> writes: >> If Luke is volunteering to take over its maintainership, it would be >> appreciated by its users. It has been in the "abandonware" status >> for too long. > > I think I am volunteering. Wonderful, and thanks. > Elsewhere in the thread, you suggested that subtree be taken out of > git.git, and live as a standalone project. > > I'm not entirely opposed to that, but > > 1. I'm not sure how whoever picks it up (me) establishes their > git-subtree as the "real" subtree (get a blessing from Avery?). Avery is so distant a past, but a work like this series, while we still have it in contrib/ in my tree, will help build necessary trust in you by the Git user/developer community, and when that happens, it would be obvious to everybody that you would be the new owner of the tool. And when that happens while the tool is in the contrib/ in my tree, and you'd be an established trusted member of the development community by then, I do not mind if you take it and maintain out of tree, if you keep working on the tool still in contrib/, or if you polish it to the main porcelain status and take it out of contrib/ and make it part of the git-core proper. > On the other hand, I think that in the long-ish term git-subtree wants > to be rewritten in a better-suited language. My personal inclination > would be Go, but if I ever want it to graduate to git-core, it'd have > to be C, huh? If somebody is willing to do a rewrite and will maintain it for a long haul, I'd say that it would not necessarily have to be in C especially if it is not performance sensitive. As long as it is done in a widely available language, that is (which used to man Perl or Python, but my persoonal preference won't carry that much weight these days ;-) Then folks who really want it in C can rewrite the rewrite on their own after that. One advantage you may have if you choose to take it out of my tree and make it a standalone project is that you have more latitude on the future implementation technology, but we'd need to start from helping you earn the trust by convincing others that "subtree" would be in good hands with your volunteering ;-) Thanks.
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index a6351d9195..74516513cd 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -11,11 +11,9 @@ and split subcommands of git subtree. TEST_DIRECTORY=$(pwd)/../../../t export TEST_DIRECTORY +. "$TEST_DIRECTORY"/test-lib.sh -. ../../../t/test-lib.sh - -subtree_test_create_repo() -{ +subtree_test_create_repo () { test_create_repo "$1" && ( cd "$1" && @@ -23,26 +21,24 @@ subtree_test_create_repo() ) } -create() -{ +create () { echo "$1" >"$1" && git add "$1" } -check_equal() -{ +check_equal () { test_debug 'echo' test_debug "echo \"check a:\" \"{$1}\"" test_debug "echo \" b:\" \"{$2}\"" - if [ "$1" = "$2" ]; then + if [ "$1" = "$2" ] + then return 0 else return 1 fi } -undo() -{ +undo () { git reset --hard HEAD~ } @@ -50,8 +46,7 @@ undo() # The original set of commits changed only one file each. # A multi-file change would imply that we pruned commits # too aggressively. -join_commits() -{ +join_commits () { commit= all= while read x y; do @@ -70,7 +65,7 @@ join_commits() echo "$commit $all" } -test_create_commit() ( +test_create_commit () ( repo=$1 && commit=$2 && cd "$repo" && @@ -81,8 +76,7 @@ test_create_commit() ( git commit -m "$commit" || error "Could not commit" ) -last_commit_message() -{ +last_commit_message () { git log --pretty=format:%s -1 } @@ -111,7 +105,8 @@ test_expect_success 'no pull from non-existent subtree' ' cd "$test_count" && git fetch ./"sub proj" HEAD && test_must_fail git subtree pull --prefix="sub dir" ./"sub proj" HEAD - )' + ) +' test_expect_success 'add subproj as subtree into sub dir/ with --prefix' ' subtree_test_create_repo "$test_count" && @@ -325,7 +320,7 @@ test_expect_success 'split sub dir/ with --rejoin' ' git subtree split --prefix="sub dir" --annotate="*" --rejoin && check_equal "$(last_commit_message)" "Split '\''sub dir/'\'' into commit '\''$split_hash'\''" ) - ' +' test_expect_success 'split sub dir/ with --rejoin from scratch' ' subtree_test_create_repo "$test_count" && @@ -340,7 +335,7 @@ test_expect_success 'split sub dir/ with --rejoin from scratch' ' git subtree split --prefix="sub dir" --rejoin && check_equal "$(last_commit_message)" "Split '\''sub dir/'\'' into commit '\''$split_hash'\''" ) - ' +' test_expect_success 'split sub dir/ with --rejoin and --message' ' subtree_test_create_repo "$test_count" && @@ -921,18 +916,18 @@ test_expect_success 'push split to subproj' ' test_create_commit "$test_count" "sub dir"/main-sub2 && ( cd $test_count/"sub proj" && - git branch sub-branch-1 && - cd .. && + git branch sub-branch-1 && + cd .. && git fetch ./"sub proj" HEAD && git subtree merge --prefix="sub dir" FETCH_HEAD ) && test_create_commit "$test_count" "sub dir"/main-sub3 && - ( + ( cd "$test_count" && - git subtree push ./"sub proj" --prefix "sub dir" sub-branch-1 && - cd ./"sub proj" && - git checkout sub-branch-1 && - check_equal "$(last_commit_message)" "sub dir/main-sub3" + git subtree push ./"sub proj" --prefix "sub dir" sub-branch-1 && + cd ./"sub proj" && + git checkout sub-branch-1 && + check_equal "$(last_commit_message)" "sub dir/main-sub3" ) '