On Thu, Feb 22, 2024 at 10:34 AM <phillip.wood123@xxxxxxxxx> wrote:
>> static int is_index_unchanged(struct repository *r)
>> {
>> - struct object_id head_oid, *cache_tree_oid;
>> + struct object_id head_oid, *cache_tree_oid, head_tree_oid;
>
> I think we can make `head_tree_oid` a pointer like cache_tree_oid and
> avoid de-referencing `the_hash_algo->empty_tree` and the return value of
> `get_commit_tree_oid()`. I think the only reason to copy it would be if
> the underlying object had a shorter lifetime than `head_tree_oid` but I
> don't think that's the case.
Makes sense -- I'll update this in v3.
>> +test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
>> + git checkout main &&
>
> I'm a bit confused by this - are we already on the branch "unborn" and
> so need to move away from it to delete it?
Yes, the previous test leaves us on that branch. In v3, I will update
this to instead just use `git checkout --detach`, as that does seem a
little less confusing than switching to some other branch that is only
relevant because it's not `unborn`. If there is a cleaner way to do
this, I'd be happy to switch to it.
>> + git branch -D unborn &&
>> + git checkout --orphan unborn &&
>> + git rm --cached -r . &&
>> + rm -rf * &&
>
> "git switch --orphan" leaves us with an empty index and working copy
> without having to remove the files ourselves.
Thanks for pointing this out. Using git-switch(1) here instead of
git-checkout(1) allows us to drop the `rm -rf *` call form both the
existing 'cherry-pick on unborn branch' test as well as my new test. It
appears that the `git rm --cached -r .` call is still necessary in the
existing test.
>> + git cherry-pick initial --allow-empty &&
>> + git diff --quiet initial &&
>
> I'd drop "--quiet" here as it makes debugging easier if we can see the
> diff if the test fails.
This makes sense. In v3, I will update this new test as well as the
existing test to not use `--quiet`.
Combining the above suggestions, here's the version of the existing and
new tests that I intend to use in v3. Let me know if this isn't what you
had in mind!
test_expect_success 'cherry-pick on unborn branch' '
git switch --orphan unborn &&
git rm --cached -r . &&
git cherry-pick initial &&
git diff initial &&
test_cmp_rev ! initial HEAD
'
test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
git checkout --detach &&
git branch -D unborn &&
git switch --orphan unborn &&
git cherry-pick initial --allow-empty &&
git diff initial &&
test_cmp_rev ! initial HEAD
'
--
Thank you,
Brian Lyles
