Re: [Outreachy] Potential intern.

[Usman Akinyemi <usmanakinyemi202@gmail.com>]

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@gmail.com> wrote:
>
> Hi Usman,
>
> On Sat, Oct 5, 2024 at 6:42 PM Usman Akinyemi
> <usmanakinyemi202@gmail.com> 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@gmail.com/
> >
> > Two email threads of discussion and feedback from maintainers.
> >
> > https://lore.kernel.org/git/CAPig+cR2-6qONkosu7=qEQSJa_fvYuVQ0to47D5qx904zW08Eg@mail.gmail.com/
> > https://public-inbox.org/git/CAPig+cRfO8t1tdCL6MB4b9XopF3HkZ==hU83AFZ38b-2zsXDjQ@mail.gmail.com/
> >
> > 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@gmail.com/
> >
> > 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@mail.gmail.com/
> > 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.