On 8/26/2020 2:46 PM, Junio C Hamano wrote: > "Sean Barag via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: >> This commit implements >> `remote.cloneDefault` as a parallel to `remote.pushDefault`, >> with prioritized name resolution: > > I highly doubt that .cloneDefault is a good name. After reading > only the title of the patch e-mail, i.e. when the only available > information on the change available to me was the name of the > configuration variable and the fact that it pertains to the command > "git clone", I thought it is to specify a URL, from which "git > clone" without the URL would clone from that single repository. > > And the name will cause the same misunderstanding to normal users, > not just to reviewers of your patch, after this change hits a future > Git release. > > Taking a parallel from init.defaultBranchName, I would probably call > it clone.defaultUpstreamName if I were writing this feature. I was thinking "clone.defaultRemoteName" makes it clear we are naming the remote for the provided <url> in the command. Having clone.defaultUpstreamName = upstream may look a bit confusing, while clone.defaultRemoteName = upstream makes it a bit clearer that we will set remote.upstream.url = <url> >> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh >> index e69427f881..8aac67b385 100755 >> --- a/t/t5606-clone-options.sh >> +++ b/t/t5606-clone-options.sh >> @@ -19,6 +19,20 @@ test_expect_success 'clone -o' ' >> >> ' >> >> +test_expect_success 'clone respects remote.cloneDefault' ' >> + >> + git -c remote.cloneDefault=bar clone parent clone-config && >> + (cd clone-config && git rev-parse --verify refs/remotes/bar/master) >> + >> +' >> + >> +test_expect_success 'clone chooses correct remote name' ' >> + >> + git -c remote.cloneDefault=bar clone -o foo parent clone-o-and-config && >> + (cd clone-o-and-config && git rev-parse --verify refs/remotes/foo/master) >> + >> +' > > These two are "showing off my shiny new toy" tests, which are > needed, but we also need negative tests where the shiny new toy does > not kick in when it should not. For example > > git -c remote.cloneDefault="bad.../...name" clone parent > > should fail, no? This is an important suggestion. Thanks, -Stolee
