Re: [PATCH 5/5] stash: make internal resets quiet and refresh index
- From
Victoria Dye <vdye@github.com>
- Date
- Mar 14, 2022, 15:56 UTC
- Message-ID
- <53967755-a8c4-d9d7-dd7b-d08782ab88ee@github.com>
- In-Reply-To
- <41db12c3-029f-2e44-703c-90cb45ed9875@github.com>
Derrick Stolee wrote:
Show 33 quoted lines
> On 3/11/2022 7:08 PM, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Add the options '-q' and '--refresh' to the 'git reset' executed in
>> 'reset_head()', and '--refresh' to the 'git reset -q' executed in
>> 'do_push_stash(...)'.
>
>> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
>> index 5e68180f3b2..f2076545133 100755
>> --- a/t/t7102-reset.sh
>> +++ b/t/t7102-reset.sh
>> @@ -482,7 +482,7 @@ test_index_refreshed () {
>> git rm --cached file2 &&
>>
>> # Step 2
>> - git reset $1 --mixed HEAD &&
>> + git reset $@ --mixed HEAD &&
>
> I see you change this from "$1" to "$@", which won't help with
> the "-c key=value" issues from earlier.
>
>>
>> # Step 3
>> git read-tree -m HEAD~1
>> @@ -491,48 +491,66 @@ test_index_refreshed () {
>> test_expect_success '--mixed refreshes the index' '
>> # Verify default behavior (with no config settings or command line
>> # options)
>> - test_index_refreshed &&
>> + test_index_refreshed
>
> Ah, I see you fixed this here, probably just a rebase issue, then.
> That's exactly what happened, and it will be fixed (and triple-checked) in V2. :)
Show 39 quoted lines
>> test_expect_success '--mixed --[no-]quiet sets default refresh behavior' ' >> # Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh) >> # determine refresh behavior >> >> - # Config setting >> - test_must_fail test_index_refreshed -c reset.quiet=true && >> - test_index_refreshed -c reset.quiet=true && >> - > > Ah, and the -c changes are removed here. You could still test them > using the trick I mention in reply to patch 2. > >> # Command line option >> - test_must_fail test_index_refreshed --quiet && >> + ! test_index_refreshed --quiet && >> test_index_refreshed --no-quiet && >> >> - # Command line option overrides config setting >> - test_must_fail test_index_refreshed -c reset.quiet=false --quiet && >> - test_index_refreshed -c reset.refresh=true --no-quiet >> + # Config: reset.quiet=false >> + test_config reset.quiet false && >> + ( >> + test_index_refreshed && >> + ! test_index_refreshed --quiet >> + ) && >> + >> + # Config: reset.quiet=true >> + test_config reset.quiet true && >> + ( >> + ! test_index_refreshed && >> + test_index_refreshed --no-quiet >> + ) > > I'm not sure why you need sub-shells here. The test_config is > not scoped to the shell. These lines could be avoided with the > -c trick, which should make it a bit simpler to show what you > intend to be testing here. >
This was mostly organizational, but I'll remove them entirely in favor of the line config option in V2.
> Thanks, > -Stolee