Hi, Thanks for your reply. I make changes but, I need someone to allow me to be able to send my patch using gitgitgadget. I had two links as the first was failing some test which I solved. Below is the github link. https://github.com/git/git/pull/1805 - updated patch. https://github.com/git/git/pull/1803 Also, I will be glad for any review of my approach on using gitgitgadget. Thank you. On Sat, Oct 5, 2024 at 6:22 PM Christian Couder <christian.couder@xxxxxxxxx> wrote: > > Hi Usman, > > On Sat, Oct 5, 2024 at 6:42 PM Usman Akinyemi > <usmanakinyemi202@xxxxxxxxx> wrote: > > > > Hi Patrick, > > > > Following this, I have gone through [1], [2] and some other resources. > > > > I was able to get potential three interesting MiniProject which I can > > work on. I have also checked through the mailing list to ensure no one > > is working on any of the particular file. As advised, I am sending > > this just to be sure if it’s worth doing and if it’s appropriate for a > > miniproject. I am sending three MiniProjects so I can have one to work > > with in case any of them is not appropriate. > > Great, thanks for your interest in working on Git! > > > 1. Use test_path_is_* functions in test scripts > > An approved sample - > > https://lore.kernel.org/git/20240304095436.56399-2-shejialuo@xxxxxxxxx/ > > > > Two email threads of discussion and feedback from maintainers. > > > > https://lore.kernel.org/git/CAPig+cR2-6qONkosu7=qEQSJa_fvYuVQ0to47D5qx904zW08Eg@xxxxxxxxxxxxxx/ > > https://public-inbox.org/git/CAPig+cRfO8t1tdCL6MB4b9XopF3HkZ==hU83AFZ38b-2zsXDjQ@xxxxxxxxxxxxxx/ > > > > Two potential test files which I saw that I can work one. > > > > t/t7003-filter-branch.sh > > > > > > test_expect_success 'test that the file was renamed' ' > > test D = "$(git show HEAD:doh --)" && > > ! test -f D.t && > > test -f doh && > > test D = "$(cat doh)" > > ' > > Yeah, this looks like a good instance where test_path_is_* functions > could be used. > > > t/t2003-checkout-cache-mkdir.sh > > > > test_expect_success 'use --prefix=path2/' ' > > rm -fr path0 path1 path2 && > > mkdir path2 && > > git checkout-index --prefix=path2/ -f -a && > > test -f path2/path0 && > > test -f path2/path1/file1 && > > test ! -f path0 && > > test ! -f path1/file1 > > ' > > This looks like another good instance. > > > These two are asserting that if a file exists, it can be changed to > > test_file_exist > > There is no test_file_exist() function in our test framework as far as > I can see. I think you should use test_path_is_file() or > test_path_is_missing() to replace `test -f ...` and ` test ! -f ...` > respectively. > > > 2. Avoid suppressing git’s exit code in test scripts > > > > Sample email thread about the same issue. > > https://public-inbox.org/git/pull.885.v2.git.git.1603032125151.gitgitgadget@xxxxxxxxx/ > > > > First file - t/t6050-replace.sh > > code sample > > test_expect_success 'replace the author' ' > > git cat-file commit $HASH2 | grep "author A U Thor" && > > R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object > > -t commit --stdin -w) && > > git cat-file commit $R | grep "author O Thor" && > > git update-ref refs/replace/$HASH2 $R && > > git show HEAD~5 | grep "O Thor" && > > git show $HASH2 | grep "O Thor" > > ' > > Yeah, I think it would be a good change to remove the pipes after git > commands in that code. > > > Second File - t/t3404-rebase-interactive.sh > > code sample that needs improvement > > > > test_expect_success 'retain authorship' ' > > echo A > file7 && > > git add file7 && > > test_tick && > > GIT_AUTHOR_NAME="Twerp Snog" git commit -m "different author" && > > git tag twerp && > > git rebase -i --onto primary HEAD^ && > > git show HEAD | grep "^Author: Twerp Snog" > > ' > > Yeah, here too, I think it would be a good change to remove the pipes > in that code. > > > 3. Modernise test. > > Description > > https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@xxxxxxxxxxxxxx/ > > Sample code t/t7611-merge-abort.sh test_expect_success 'Reset index > > (but preserve worktree changes)' ' > > git reset "$pre_merge_head" && > > git diff > actual && > > test_cmp expect actual > > ' > > From the sample, it's not clear that the code would benefit a lot from > being modernized, so I would recommend focusing on improving one of > the other code you mention above. > > Thanks, > Christian.
