| Message ID | 20220123060318.471414-1-shaoxuan.yuan02@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GSoC] lib-read-tree-m-3way: modernize a test script (style) | expand |
Not sure if this email is overlooked (I understand its content is lackluster though). I'm looking forward to participating in this year's GSoC :) And I will be really appreciated if anyone could possibly reply to it (and my self-intro). -- Thanks, Shaoxuan On Sun, Jan 23, 2022 at 2:04 PM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote: > > As a microproject, I found another small fix regarding styling to do. > > I changed the old style '\' (backslash) to new style "'" (single > quotes). > > And I also fixed some double quotes misuse. > > Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> > --- > Other than that, I forgot to introduce myself in the last patch and > here it goes: > > I'm Shaoxuan Yuan, currently a sophomore majors in Computer Science and Engineering > (CSE) @ University of California, Irvine. > > I have prior open-source experience in which I was [maintaining|contributing to] the > Casbin community. My main language is Python, and I'm a C newbie > because I'm quite interested in contributing to git (since it is in my main daily > toolkit and it is a charm to wield :-) ). > > And for now I'm still taking baby steps trying to crack some test script > styling issues. After getting more familiar with the git contribution > process, I will try something bigger (though not THAT big) to get a > firmer grasp of git. > > t/lib-read-tree-m-3way.sh | 154 +++++++++++++++++++------------------- > 1 file changed, 77 insertions(+), 77 deletions(-) > > diff --git a/t/lib-read-tree-m-3way.sh b/t/lib-read-tree-m-3way.sh > index 168329adbc..e40739b8db 100644 > --- a/t/lib-read-tree-m-3way.sh > +++ b/t/lib-read-tree-m-3way.sh > @@ -8,16 +8,16 @@ do > p=$a$b > echo This is $p from the original tree. >$p > echo This is Z/$p from the original tree. >Z/$p > - test_expect_success \ > - "adding test file $p and Z/$p" \ > - 'git update-index --add $p && > - git update-index --add Z/$p' > + test_expect_success 'adding test file $p and Z/$p' ' > + git update-index --add $p && > + git update-index --add Z/$p > + ' > done > done > echo This is SS from the original tree. >SS > -test_expect_success \ > - 'adding test file SS' \ > - 'git update-index --add SS' > +test_expect_success 'adding test file SS' ' > + git update-index --add SS > +' > cat >TT <<\EOF > This is a trivial merge sample text. > Branch A is expected to upcase this word, here. > @@ -30,12 +30,12 @@ At the very end, here comes another line, that is > the word, expected to be upcased by Branch B. > This concludes the trivial merge sample file. > EOF > -test_expect_success \ > - 'adding test file TT' \ > - 'git update-index --add TT' > -test_expect_success \ > - 'prepare initial tree' \ > - 'tree_O=$(git write-tree)' > +test_expect_success 'adding test file TT' ' > + git update-index --add TT > +' > +test_expect_success 'prepare initial tree' ' > + tree_O=$(git write-tree) > +' > > ################################################################ > # Branch A and B makes the changes according to the above matrix. > @@ -45,48 +45,48 @@ test_expect_success \ > > to_remove=$(echo D? Z/D?) > rm -f $to_remove > -test_expect_success \ > - 'change in branch A (removal)' \ > - 'git update-index --remove $to_remove' > +test_expect_success 'change in branch A (removal)' ' > + git update-index --remove $to_remove > +' > > for p in M? Z/M? > do > echo This is modified $p in the branch A. >$p > - test_expect_success \ > - 'change in branch A (modification)' \ > - "git update-index $p" > + test_expect_success 'change in branch A (modification)' ' > + git update-index $p > + ' > done > > for p in AN AA Z/AN Z/AA > do > echo This is added $p in the branch A. >$p > - test_expect_success \ > - 'change in branch A (addition)' \ > - "git update-index --add $p" > + test_expect_success 'change in branch A (addition)' ' > + git update-index --add $p > + ' > done > > echo This is SS from the modified tree. >SS > echo This is LL from the modified tree. >LL > -test_expect_success \ > - 'change in branch A (addition)' \ > - 'git update-index --add LL && > - git update-index SS' > +test_expect_success 'change in branch A (addition)' ' > + git update-index --add LL && > + git update-index SS > +' > mv TT TT- > sed -e '/Branch A/s/word/WORD/g' <TT- >TT > rm -f TT- > -test_expect_success \ > - 'change in branch A (edit)' \ > - 'git update-index TT' > +test_expect_success 'change in branch A (edit)' ' > + git update-index TT > +' > > mkdir DF > echo Branch A makes a file at DF/DF, creating a directory DF. >DF/DF > -test_expect_success \ > - 'change in branch A (change file to directory)' \ > - 'git update-index --add DF/DF' > +test_expect_success 'change in branch A (change file to directory)' ' > + git update-index --add DF/DF > +' > > -test_expect_success \ > - 'recording branch A tree' \ > - 'tree_A=$(git write-tree)' > +test_expect_success 'recording branch A tree' ' > + tree_A=$(git write-tree) > +' > > ################################################################ > # Branch B > @@ -94,65 +94,65 @@ test_expect_success \ > > rm -rf [NDMASLT][NDMASLT] Z DF > mkdir Z > -test_expect_success \ > - 'reading original tree and checking out' \ > - 'git read-tree $tree_O && > - git checkout-index -a' > +test_expect_success 'reading original tree and checking out' ' > + git read-tree $tree_O && > + git checkout-index -a > +' > > to_remove=$(echo ?D Z/?D) > rm -f $to_remove > -test_expect_success \ > - 'change in branch B (removal)' \ > - "git update-index --remove $to_remove" > +test_expect_success 'change in branch B (removal)' ' > + git update-index --remove $to_remove > +' > > for p in ?M Z/?M > do > echo This is modified $p in the branch B. >$p > - test_expect_success \ > - 'change in branch B (modification)' \ > - "git update-index $p" > + test_expect_success 'change in branch B (modification)' ' > + git update-index $p > + ' > done > > for p in NA AA Z/NA Z/AA > do > echo This is added $p in the branch B. >$p > - test_expect_success \ > - 'change in branch B (addition)' \ > - "git update-index --add $p" > + test_expect_success 'change in branch B (addition)' ' > + git update-index --add $p > + ' > done > echo This is SS from the modified tree. >SS > echo This is LL from the modified tree. >LL > -test_expect_success \ > - 'change in branch B (addition and modification)' \ > - 'git update-index --add LL && > - git update-index SS' > +test_expect_success 'change in branch B (addition and modification)' ' > + git update-index --add LL && > + git update-index SS > +' > mv TT TT- > sed -e '/Branch B/s/word/WORD/g' <TT- >TT > rm -f TT- > -test_expect_success \ > - 'change in branch B (modification)' \ > - 'git update-index TT' > +test_expect_success 'change in branch B (modification)' ' > + git update-index TT > +' > > echo Branch B makes a file at DF. >DF > -test_expect_success \ > - 'change in branch B (addition of a file to conflict with directory)' \ > - 'git update-index --add DF' > - > -test_expect_success \ > - 'recording branch B tree' \ > - 'tree_B=$(git write-tree)' > - > -test_expect_success \ > - 'keep contents of 3 trees for easy access' \ > - 'rm -f .git/index && > - git read-tree $tree_O && > - mkdir .orig-O && > - git checkout-index --prefix=.orig-O/ -f -q -a && > - rm -f .git/index && > - git read-tree $tree_A && > - mkdir .orig-A && > - git checkout-index --prefix=.orig-A/ -f -q -a && > - rm -f .git/index && > - git read-tree $tree_B && > - mkdir .orig-B && > - git checkout-index --prefix=.orig-B/ -f -q -a' > +test_expect_success 'change in branch B (addition of a file to conflict with directory)' ' > + git update-index --add DF > +' > + > +test_expect_success 'recording branch B tree' ' > + tree_B=$(git write-tree) > +' > + > +test_expect_success 'keep contents of 3 trees for easy access' ' > + rm -f .git/index && > + git read-tree $tree_O && > + mkdir .orig-O && > + git checkout-index --prefix=.orig-O/ -f -q -a && > + rm -f .git/index && > + git read-tree $tree_A && > + mkdir .orig-A && > + git checkout-index --prefix=.orig-A/ -f -q -a && > + rm -f .git/index && > + git read-tree $tree_B && > + mkdir .orig-B && > + git checkout-index --prefix=.orig-B/ -f -q -a > +' > -- > 2.25.1 >
On Mon, Jan 24, 2022 at 1:37 AM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote: > lib-read-tree-m-3way: modernize a test script (style) Prefixing the subject with "t/" would help better explain which part of the project this patch is touching. Perhaps: t/lib-read-tree-m-3way: modernize style > As a microproject, I found another small fix regarding styling to do. Everything above the "---" line below your sign-off will become part of the permanent project history; everything below the "---" is mere commentary for reviewers. Reviewers may be interested in knowing that this is a microproject, but it likely won't be meaningful to future readers of the project history. As such, place this sort of commentary below the "---" line. > I changed the old style '\' (backslash) to new style "'" (single > quotes). Not sure what this means. I _think_ you mean that you changed: test_expect_success \ 'title' \ 'body' to: test_expect_success 'title' ' body ' Sometimes you can convey such a meaning more clearly by a simple example as illustrated above. > And I also fixed some double quotes misuse. Again, it is unclear what this means. Providing an example can help readers understand what you changed. > Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> > --- > Other than that, I forgot to introduce myself in the last patch and > here it goes: > > I'm Shaoxuan Yuan, currently a sophomore majors in Computer Science and Engineering > (CSE) @ University of California, Irvine. > > I have prior open-source experience in which I was [maintaining|contributing to] the > Casbin community. My main language is Python, and I'm a C newbie > because I'm quite interested in contributing to git (since it is in my main daily > toolkit and it is a charm to wield :-) ). > > And for now I'm still taking baby steps trying to crack some test script > styling issues. After getting more familiar with the git contribution > process, I will try something bigger (though not THAT big) to get a > firmer grasp of git. Welcome. Please understand that all review comments (above and below) are meant to be constructive and help familiarize you with the local customs of the Git project; they are not meant as any sort of criticism. > diff --git a/t/lib-read-tree-m-3way.sh b/t/lib-read-tree-m-3way.sh > @@ -8,16 +8,16 @@ do > echo This is Z/$p from the original tree. >Z/$p > - test_expect_success \ > - "adding test file $p and Z/$p" \ > - 'git update-index --add $p && > - git update-index --add Z/$p' > + test_expect_success 'adding test file $p and Z/$p' ' > + git update-index --add $p && > + git update-index --add Z/$p > + ' Taking into consideration the difference in behavior of single-quoted strings and double-quoted strings, changing this test's title from double- to single-quoted is undesirable since `$p` is supposed to be interpolated into the title; the title should not contain a literal "$p". (Unlike the test title which is simply echo'd/print'd, the test body gets eval'd, which is why `$p` in the body is acceptable even though the body is in single-quotes.) > -test_expect_success \ > - 'adding test file SS' \ > - 'git update-index --add SS' > +test_expect_success 'adding test file SS' ' > + git update-index --add SS > +' Since you're touching this anyhow as part of fixing style issues, you should also fix the indentation to use tabs rather than spaces. The same comment applies to the rest of the patch, as well. > for p in M? Z/M? > do > echo This is modified $p in the branch A. >$p > - test_expect_success \ > - 'change in branch A (modification)' \ > - "git update-index $p" > + test_expect_success 'change in branch A (modification)' ' > + git update-index $p > + ' > done In this case, the indentation of the entire body of the for-loop needs to be fixed to use tabs rather than spaces, however, fixing all the indentation problems together with the other problems can make for a too-noisy patch for reviewers to really see what is going on. As such, it may be better to make this a multi-patch series in which one patch fixes indentation problems only. As mentioned above, changing the body of the test from double- to single-quoted string should (presumably) be okay since the body gets eval'd and `$p` will be visible at the time of `eval`, however, mixing this sort of change in with all the other changes being made makes it hard for reviewers to spot such little details, let alone reason about correctness. As such, switching of quote types is also probably the sort of change which should be split out into its own patch. The same comment applies to other changes following this one. Overall, with the exception of the test title which needs to interpolate `$p`, the rest of the changes are probably reasonable and benign. It's important to understand that lengthy patches like this full of mechanical changes tend to be quite taxing on reviewers, so it's a good idea to help in any way you can to ease the review burden. This can be done, for instance, by making only a single type of change per patch (i.e. indentation fixes), or by limiting the scope or breadth of the changes, which is especially important for this sort of "noise" change -- a change just for the sake of change -- which doesn't have any immediate practical benefit.
On Fri, Jan 28, 2022 at 4:34 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Mon, Jan 24, 2022 at 1:37 AM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote: > > lib-read-tree-m-3way: modernize a test script (style) > > Prefixing the subject with "t/" would help better explain which part > of the project this patch is touching. Perhaps: > > t/lib-read-tree-m-3way: modernize style Thanks, this tip is really helpful and I forgot to add the directory prefix. > > As a microproject, I found another small fix regarding styling to do. > > Everything above the "---" line below your sign-off will become part > of the permanent project history; everything below the "---" is mere > commentary for reviewers. Reviewers may be interested in knowing that > this is a microproject, but it likely won't be meaningful to future > readers of the project history. As such, place this sort of commentary > below the "---" line. Sure, I did not realize this. I should've put the comments below "---" instead. > > I changed the old style '\' (backslash) to new style "'" (single > > quotes). > > Not sure what this means. I _think_ you mean that you changed: > > test_expect_success \ > 'title' \ > 'body' > > to: > > test_expect_success 'title' ' > body > ' > > Sometimes you can convey such a meaning more clearly by a simple > example as illustrated above. > > > And I also fixed some double quotes misuse. > > Again, it is unclear what this means. Providing an example can help > readers understand what you changed. Will do, definitely a helpful tip. > > Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> > > --- > > Other than that, I forgot to introduce myself in the last patch and > > here it goes: > > > > I'm Shaoxuan Yuan, currently a sophomore majors in Computer Science and Engineering > > (CSE) @ University of California, Irvine. > > > > I have prior open-source experience in which I was [maintaining|contributing to] the > > Casbin community. My main language is Python, and I'm a C newbie > > because I'm quite interested in contributing to git (since it is in my main daily > > toolkit and it is a charm to wield :-) ). > > > > And for now I'm still taking baby steps trying to crack some test script > > styling issues. After getting more familiar with the git contribution > > process, I will try something bigger (though not THAT big) to get a > > firmer grasp of git. > > Welcome. Please understand that all review comments (above and below) > are meant to be constructive and help familiarize you with the local > customs of the Git project; they are not meant as any sort of > criticism. Thanks, I learned a lot from your review comments :) > > diff --git a/t/lib-read-tree-m-3way.sh b/t/lib-read-tree-m-3way.sh > > @@ -8,16 +8,16 @@ do > > echo This is Z/$p from the original tree. >Z/$p > > - test_expect_success \ > > - "adding test file $p and Z/$p" \ > > - 'git update-index --add $p && > > - git update-index --add Z/$p' > > + test_expect_success 'adding test file $p and Z/$p' ' > > + git update-index --add $p && > > + git update-index --add Z/$p > > + ' > > Taking into consideration the difference in behavior of single-quoted > strings and double-quoted strings, changing this test's title from > double- to single-quoted is undesirable since `$p` is supposed to be > interpolated into the title; the title should not contain a literal > "$p". (Unlike the test title which is simply echo'd/print'd, the test > body gets eval'd, which is why `$p` in the body is acceptable even > though the body is in single-quotes.) Agree, I didn't realize the difference between single and double quotes in shell scripts (I was just imitating other similar style fixes). > > -test_expect_success \ > > - 'adding test file SS' \ > > - 'git update-index --add SS' > > +test_expect_success 'adding test file SS' ' > > + git update-index --add SS > > +' > > Since you're touching this anyhow as part of fixing style issues, you > should also fix the indentation to use tabs rather than spaces. The > same comment applies to the rest of the patch, as well. Sure. > > for p in M? Z/M? > > do > > echo This is modified $p in the branch A. >$p > > - test_expect_success \ > > - 'change in branch A (modification)' \ > > - "git update-index $p" > > + test_expect_success 'change in branch A (modification)' ' > > + git update-index $p > > + ' > > done > > In this case, the indentation of the entire body of the for-loop needs > to be fixed to use tabs rather than spaces, however, fixing all the > indentation problems together with the other problems can make for a > too-noisy patch for reviewers to really see what is going on. As such, > it may be better to make this a multi-patch series in which one patch > fixes indentation problems only. > As mentioned above, changing the body of the test from double- to > single-quoted string should (presumably) be okay since the body gets > eval'd and `$p` will be visible at the time of `eval`, however, mixing > this sort of change in with all the other changes being made makes it > hard for reviewers to spot such little details, let alone reason about > correctness. As such, switching of quote types is also probably the > sort of change which should be split out into its own patch. The same > comment applies to other changes following this one. I think so. I was thinking fixing all the general styling issues in one patch (since they are all style related), but now I realize that the general style patch can be divided into sub-patches for clearer review experience. And my question is, should I do this "multi-patch series" thing in the form of "-v<n>" (all under this thread), e.g. "v2" or "v3"? Or I just submit multiple patches separately (a new thread for each one)? > Overall, with the exception of the test title which needs to > interpolate `$p`, the rest of the changes are probably reasonable and > benign. It's important to understand that lengthy patches like this > full of mechanical changes tend to be quite taxing on reviewers, so > it's a good idea to help in any way you can to ease the review burden. > This can be done, for instance, by making only a single type of change > per patch (i.e. indentation fixes), or by limiting the scope or > breadth of the changes, which is especially important for this sort of I'm not quite sure what this means, and I quote, "limiting the scope or breadth of the changes". Are you suggesting, for example, fixing fewer occurrences of tab indentation issue in a patch; or, for example, limiting the fix to a specific "test_expect_success" block, and do multiple patches sequentially? > "noise" change -- a change just for the sake of change -- which > doesn't have any immediate practical benefit. Overall, thanks for the reply and it is really helpful! I did make a lot of mistakes in this patch, but I'm learning a lot :) And I will amend my patch base on these suggestions then submit a v2. -- Thanks, Shaoxuan
On Fri, Jan 28, 2022 at 4:51 AM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote: > On Fri, Jan 28, 2022 at 4:34 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > > In this case, the indentation of the entire body of the for-loop needs > > to be fixed to use tabs rather than spaces, however, fixing all the > > indentation problems together with the other problems can make for a > > too-noisy patch for reviewers to really see what is going on. As such, > > it may be better to make this a multi-patch series in which one patch > > fixes indentation problems only. > > > As mentioned above, changing the body of the test from double- to > > single-quoted string should (presumably) be okay since the body gets > > eval'd and `$p` will be visible at the time of `eval`, however, mixing > > this sort of change in with all the other changes being made makes it > > hard for reviewers to spot such little details, let alone reason about > > correctness. As such, switching of quote types is also probably the > > sort of change which should be split out into its own patch. The same > > comment applies to other changes following this one. > > I think so. I was thinking fixing all the general styling issues in one > patch (since they are all style related), but now I realize that the general > style patch can be divided into sub-patches for clearer review experience. > > And my question is, should I do this "multi-patch series" thing in the form of > "-v<n>" (all under this thread), e.g. "v2" or "v3"? Or I just submit > multiple patches separately (a new thread for each one)? A multi-patch series as v2, v3, etc. would indeed be appropriate, as you already figured out[1] before I got around to answering belatedly. > > Overall, with the exception of the test title which needs to > > interpolate `$p`, the rest of the changes are probably reasonable and > > benign. It's important to understand that lengthy patches like this > > full of mechanical changes tend to be quite taxing on reviewers, so > > it's a good idea to help in any way you can to ease the review burden. > > This can be done, for instance, by making only a single type of change > > per patch (i.e. indentation fixes), or by limiting the scope or > > breadth of the changes, which is especially important for this sort of > > I'm not quite sure what this means, and I quote, "limiting the scope or > breadth of the changes". Are you suggesting, for example, > fixing fewer occurrences of tab indentation issue in a patch; or, for > example, limiting the > fix to a specific "test_expect_success" block, and do multiple patches > sequentially? Sorry for being unclear. I just meant that as a microproject, it would have worked equally well to pick a much smaller test script with style problems (if you could find one) rather than a long script. After all, the purpose of a microproject is to give you experience with the submission-review process and to give reviewers and mentors an idea of how you interact. It's the process which is important, in this case, not the size of the submission. Anyhow, it looks like Junio is happy with your v3 and will be merging it down to "next", so it all worked out. [1]: https://lore.kernel.org/git/20220202064300.3601-1-shaoxuan.yuan02@gmail.com/ [2]: https://lore.kernel.org/git/xmqqr18jnr2t.fsf@gitster.g/
Got it, I'm learning along the way, and thanks for the reply! -- Thanks, Shaoxuan On Sat, Feb 5, 2022 at 7:59 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Fri, Jan 28, 2022 at 4:51 AM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote: > > On Fri, Jan 28, 2022 at 4:34 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > > > In this case, the indentation of the entire body of the for-loop needs > > > to be fixed to use tabs rather than spaces, however, fixing all the > > > indentation problems together with the other problems can make for a > > > too-noisy patch for reviewers to really see what is going on. As such, > > > it may be better to make this a multi-patch series in which one patch > > > fixes indentation problems only. > > > > > As mentioned above, changing the body of the test from double- to > > > single-quoted string should (presumably) be okay since the body gets > > > eval'd and `$p` will be visible at the time of `eval`, however, mixing > > > this sort of change in with all the other changes being made makes it > > > hard for reviewers to spot such little details, let alone reason about > > > correctness. As such, switching of quote types is also probably the > > > sort of change which should be split out into its own patch. The same > > > comment applies to other changes following this one. > > > > I think so. I was thinking fixing all the general styling issues in one > > patch (since they are all style related), but now I realize that the general > > style patch can be divided into sub-patches for clearer review experience. > > > > And my question is, should I do this "multi-patch series" thing in the form of > > "-v<n>" (all under this thread), e.g. "v2" or "v3"? Or I just submit > > multiple patches separately (a new thread for each one)? > > A multi-patch series as v2, v3, etc. would indeed be appropriate, as > you already figured out[1] before I got around to answering belatedly. > > > > Overall, with the exception of the test title which needs to > > > interpolate `$p`, the rest of the changes are probably reasonable and > > > benign. It's important to understand that lengthy patches like this > > > full of mechanical changes tend to be quite taxing on reviewers, so > > > it's a good idea to help in any way you can to ease the review burden. > > > This can be done, for instance, by making only a single type of change > > > per patch (i.e. indentation fixes), or by limiting the scope or > > > breadth of the changes, which is especially important for this sort of > > > > I'm not quite sure what this means, and I quote, "limiting the scope or > > breadth of the changes". Are you suggesting, for example, > > fixing fewer occurrences of tab indentation issue in a patch; or, for > > example, limiting the > > fix to a specific "test_expect_success" block, and do multiple patches > > sequentially? > > Sorry for being unclear. I just meant that as a microproject, it would > have worked equally well to pick a much smaller test script with style > problems (if you could find one) rather than a long script. After all, > the purpose of a microproject is to give you experience with the > submission-review process and to give reviewers and mentors an idea of > how you interact. It's the process which is important, in this case, > not the size of the submission. > > Anyhow, it looks like Junio is happy with your v3 and will be merging > it down to "next", so it all worked out. > > [1]: https://lore.kernel.org/git/20220202064300.3601-1-shaoxuan.yuan02@gmail.com/ > [2]: https://lore.kernel.org/git/xmqqr18jnr2t.fsf@gitster.g/
diff --git a/t/lib-read-tree-m-3way.sh b/t/lib-read-tree-m-3way.sh index 168329adbc..e40739b8db 100644 --- a/t/lib-read-tree-m-3way.sh +++ b/t/lib-read-tree-m-3way.sh @@ -8,16 +8,16 @@ do p=$a$b echo This is $p from the original tree. >$p echo This is Z/$p from the original tree. >Z/$p - test_expect_success \ - "adding test file $p and Z/$p" \ - 'git update-index --add $p && - git update-index --add Z/$p' + test_expect_success 'adding test file $p and Z/$p' ' + git update-index --add $p && + git update-index --add Z/$p + ' done done echo This is SS from the original tree. >SS -test_expect_success \ - 'adding test file SS' \ - 'git update-index --add SS' +test_expect_success 'adding test file SS' ' + git update-index --add SS +' cat >TT <<\EOF This is a trivial merge sample text. Branch A is expected to upcase this word, here. @@ -30,12 +30,12 @@ At the very end, here comes another line, that is the word, expected to be upcased by Branch B. This concludes the trivial merge sample file. EOF -test_expect_success \ - 'adding test file TT' \ - 'git update-index --add TT' -test_expect_success \ - 'prepare initial tree' \ - 'tree_O=$(git write-tree)' +test_expect_success 'adding test file TT' ' + git update-index --add TT +' +test_expect_success 'prepare initial tree' ' + tree_O=$(git write-tree) +' ################################################################ # Branch A and B makes the changes according to the above matrix. @@ -45,48 +45,48 @@ test_expect_success \ to_remove=$(echo D? Z/D?) rm -f $to_remove -test_expect_success \ - 'change in branch A (removal)' \ - 'git update-index --remove $to_remove' +test_expect_success 'change in branch A (removal)' ' + git update-index --remove $to_remove +' for p in M? Z/M? do echo This is modified $p in the branch A. >$p - test_expect_success \ - 'change in branch A (modification)' \ - "git update-index $p" + test_expect_success 'change in branch A (modification)' ' + git update-index $p + ' done for p in AN AA Z/AN Z/AA do echo This is added $p in the branch A. >$p - test_expect_success \ - 'change in branch A (addition)' \ - "git update-index --add $p" + test_expect_success 'change in branch A (addition)' ' + git update-index --add $p + ' done echo This is SS from the modified tree. >SS echo This is LL from the modified tree. >LL -test_expect_success \ - 'change in branch A (addition)' \ - 'git update-index --add LL && - git update-index SS' +test_expect_success 'change in branch A (addition)' ' + git update-index --add LL && + git update-index SS +' mv TT TT- sed -e '/Branch A/s/word/WORD/g' <TT- >TT rm -f TT- -test_expect_success \ - 'change in branch A (edit)' \ - 'git update-index TT' +test_expect_success 'change in branch A (edit)' ' + git update-index TT +' mkdir DF echo Branch A makes a file at DF/DF, creating a directory DF. >DF/DF -test_expect_success \ - 'change in branch A (change file to directory)' \ - 'git update-index --add DF/DF' +test_expect_success 'change in branch A (change file to directory)' ' + git update-index --add DF/DF +' -test_expect_success \ - 'recording branch A tree' \ - 'tree_A=$(git write-tree)' +test_expect_success 'recording branch A tree' ' + tree_A=$(git write-tree) +' ################################################################ # Branch B @@ -94,65 +94,65 @@ test_expect_success \ rm -rf [NDMASLT][NDMASLT] Z DF mkdir Z -test_expect_success \ - 'reading original tree and checking out' \ - 'git read-tree $tree_O && - git checkout-index -a' +test_expect_success 'reading original tree and checking out' ' + git read-tree $tree_O && + git checkout-index -a +' to_remove=$(echo ?D Z/?D) rm -f $to_remove -test_expect_success \ - 'change in branch B (removal)' \ - "git update-index --remove $to_remove" +test_expect_success 'change in branch B (removal)' ' + git update-index --remove $to_remove +' for p in ?M Z/?M do echo This is modified $p in the branch B. >$p - test_expect_success \ - 'change in branch B (modification)' \ - "git update-index $p" + test_expect_success 'change in branch B (modification)' ' + git update-index $p + ' done for p in NA AA Z/NA Z/AA do echo This is added $p in the branch B. >$p - test_expect_success \ - 'change in branch B (addition)' \ - "git update-index --add $p" + test_expect_success 'change in branch B (addition)' ' + git update-index --add $p + ' done echo This is SS from the modified tree. >SS echo This is LL from the modified tree. >LL -test_expect_success \ - 'change in branch B (addition and modification)' \ - 'git update-index --add LL && - git update-index SS' +test_expect_success 'change in branch B (addition and modification)' ' + git update-index --add LL && + git update-index SS +' mv TT TT- sed -e '/Branch B/s/word/WORD/g' <TT- >TT rm -f TT- -test_expect_success \ - 'change in branch B (modification)' \ - 'git update-index TT' +test_expect_success 'change in branch B (modification)' ' + git update-index TT +' echo Branch B makes a file at DF. >DF -test_expect_success \ - 'change in branch B (addition of a file to conflict with directory)' \ - 'git update-index --add DF' - -test_expect_success \ - 'recording branch B tree' \ - 'tree_B=$(git write-tree)' - -test_expect_success \ - 'keep contents of 3 trees for easy access' \ - 'rm -f .git/index && - git read-tree $tree_O && - mkdir .orig-O && - git checkout-index --prefix=.orig-O/ -f -q -a && - rm -f .git/index && - git read-tree $tree_A && - mkdir .orig-A && - git checkout-index --prefix=.orig-A/ -f -q -a && - rm -f .git/index && - git read-tree $tree_B && - mkdir .orig-B && - git checkout-index --prefix=.orig-B/ -f -q -a' +test_expect_success 'change in branch B (addition of a file to conflict with directory)' ' + git update-index --add DF +' + +test_expect_success 'recording branch B tree' ' + tree_B=$(git write-tree) +' + +test_expect_success 'keep contents of 3 trees for easy access' ' + rm -f .git/index && + git read-tree $tree_O && + mkdir .orig-O && + git checkout-index --prefix=.orig-O/ -f -q -a && + rm -f .git/index && + git read-tree $tree_A && + mkdir .orig-A && + git checkout-index --prefix=.orig-A/ -f -q -a && + rm -f .git/index && + git read-tree $tree_B && + mkdir .orig-B && + git checkout-index --prefix=.orig-B/ -f -q -a +'
As a microproject, I found another small fix regarding styling to do. I changed the old style '\' (backslash) to new style "'" (single quotes). And I also fixed some double quotes misuse. Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> --- Other than that, I forgot to introduce myself in the last patch and here it goes: I'm Shaoxuan Yuan, currently a sophomore majors in Computer Science and Engineering (CSE) @ University of California, Irvine. I have prior open-source experience in which I was [maintaining|contributing to] the Casbin community. My main language is Python, and I'm a C newbie because I'm quite interested in contributing to git (since it is in my main daily toolkit and it is a charm to wield :-) ). And for now I'm still taking baby steps trying to crack some test script styling issues. After getting more familiar with the git contribution process, I will try something bigger (though not THAT big) to get a firmer grasp of git. t/lib-read-tree-m-3way.sh | 154 +++++++++++++++++++------------------- 1 file changed, 77 insertions(+), 77 deletions(-)