On Mon, Sep 17, 2012 at 10:18 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Ralf Thielow <ralf.thielow@xxxxxxxxx> writes: > >> - handle --mirror option (test added) > > Handle how? I personally think erroring out is the right way to > handle it, but if we care about people who have been misusing the > combination of single and mirror, the second best way would be to > imply "mirror" and "single" combination as "bare" and "single" > without "mirror". > With this patch it would be "bare" and "single", and with the next fetch a "mirror". However, the combination of "single" and "mirror" doesn't really make sense so I also think we should error it out. >> - install correct refspec if the value of --branch is a tag (test added) > > What is the definition of "correct"? I see the documentation says > "--branch can also take tags and treat them like detached HEAD", and > even though I _think_ allowing tags was a huge mistake, if we claim > we treat it like detached HEAD, we should do the same when coming up > with the refspec used for the follow-up "fetch". > This patch would install the refspec "+refs/tags/v1.7.10:refs/tags/v1.7.10", so we would do the same with the follow-up "fetch", not? This can be seen as "it's not a bug, it's a feature". Why not cloning a tag of a repo if someone just want to build a tag without having anything else. > Whatever we decide to do, the semantics we decided to use at least > need to be described in the commit log message, not just in the > "changes from the previous iteration". Updating the > "Documentation/git-clone.txt" would also be necessary. > Ok. >> +test_expect_success 'refspec contains all branches by default' ' >> + echo "+refs/heads/*:refs/remotes/origin/*" > expect && >> + git --git-dir=dir_all/.git config --get remote.origin.fetch > actual && >> + test_cmp expect actual >> +' > > I still think these checks that know the current implementation > details (i.e. what exact configuration variables get what exact > values) are wrong thing to have in the longer term. If the desired > behaviour (i.e. "later fetch do not screw up") can be tested > directly like the later parts of the test in this patch does, how > that desired behaviour is implemented should not have to be cast in > stone with these tests. You wrote > I'll let it pass for now, though. so I haven't removed them yet. I'll delete them. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html