On Sat, Apr 27, 2019 at 08:07:34AM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@xxxxxxxxx> writes: > > > Thanks for your comments, Eric and Junio. > > > > Eric, I've combined the `test_when_finished` calls together so that the > > statements within appear in a more "logical" order. > > > > Junio, I've taken your suggestion and moved the change into > > `create_branch`. Initially, I didn't want to do this because I didn't > > want to change the semantics of git-branch but introducing the merge > > base syntax seems to be a positive change so let's do it. > > ... > > Denton Liu (3): > > t2018: cleanup in current test > > t2018: demonstrate checkout -b merge base bug > > branch: make create_branch accept a merge base rev > > Because "checkout -b new" is supposed to be merely a short-hand for > a "branch new" followed by "checkout new", the lack of "branch new > A...B" is the same "bug" as the lack of "checkout -b new A...B". I didn't consider it to be the same "bug" because git-checkout.txt mentions the "..." syntax, whereas it isn't mentioned in git-branch.txt. That being said, now that I look at git-checkout.txt again, when doing a "regular" checkout, the parameter is called <branch> whereas when we're doing a checkout -b, it's called <start_point>, which doesn't mention "...". > > The second patch that does not talk about the former but singles out > only the latter is being inconsistent. > > One person's lack of feature is a bug to another person, and indeed, > when we did "checkout A...B" in 2009, we weren't interested in doing > the same for "checkout -b new", and nobody thought about adding that > until now, and/or considered the lack of feature as a bug. I agree, based on the above, I now see that it's a lack of feature and not a bug. > > We do not "demonstrate" the lack of a new feature in a patch with > expect-failure, followed by another patch that adds the feature that > flips expect-failure to expect-success. A patch that teaches > "checkout -b" about A...B, that is adding a missing feature, should > not have to do so. As it is shades of gray between a change being a > bugfix and adding a new feature, switching the style of testing > based on the distinction between them does not make much sense. Be > consistent and stick to just one style. And having the test and the > code change (be it adding a missing feature or fixing a bug) in a > single patch makes patch management a lot simpler by making it > harder to lose only one half. > > Having a preliminary clean-up as a separate step is a good idea, but > for this topic, I think the latter two should be combined into a > single patch that changes the code and adds tests at the same time. I'm going send a reroll to update the documentation to mention "..." in <start_point> and, while I'm at it, I'll do the squash. Thanks for your comments, Denton > > Thanks.
