Re: [PATCH v2 1/3] fsck: verify commit graph when implicitly enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> Because I took the time to scan backward through this test script, I 
>> understand that `core.commitGraph` is already `true` by the time this 
>> test is reached, thus the new test title is accurate (for now). However, 
>> it would be a bit easier to reason about this and make it more robust by 
>> having the test itself guarantee that it is set to `true` by invoking 
>> `git config core.commitGraph true`.

>> test_config() unsets the config variable when the test completes, so I'm 
>> wondering if its use is in fact desirable here. I ask because, from a 
>> quick scan through the file, it appears that many tests in this script 
>> assume that `core.commitGraph` is `true`, so it's not clear that 
>> unsetting it at the end of this test is a good idea in general.
>
> This is a good point. I made the original change in response to Taylor's
> comment, but I think we both didn't consider that this script assumes
> `core.commitGraph` is `true`. The rest of the tests pass, but only
> because the default value is true and I'd rather not have tests rely on
> a happy accident.

I said I would incorporate these suggestions, but I didn't propose what
changes I would actually make.

Given that each test depends on a global config value in the setup
phase, it might be simplest to read if we try to avoid anything that
touches that value. The easiest way to achieve this is to use git -c
core.commitGraph {true,false} for the {true,false} cases. Since there is
no -c equivalent for the unset case, I'll continue to use
test_unconfig() + test_when_finished() to temporarily unset the value.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux