Thanks, Junio for raising all these important questions. Indeed, showing tests in order to explain my thinking about the feature was a bad idea. I realise that I should have explained the feature first instead of getting the tests reviewed without any elaboration of the intentions. I will explain it now. My definition of "copy" for this feature is "copying from A to create B, keeping A intact". That means "copy branch A to B" should do whatever "move branch A to B" does except it shouldn't delete A and should keep A unchanged. 1. When a branch topic-2 is created by copying from branch topic-1, topic-2 branch reflog should now contain the all the entries of topic-1 branch (before copying) followed by "Copied from topic-1". [This is debatable though, I want inputs/suggestions about this.] 2. Copying a branch should also copy the git config section for that branch. This means if topic-2 branch is created from topic-1, "branch.topic-2.remote" should now be same as "branch.topic-1.remote", if set. 3. "git push" to copied branch for example - topic-2 should push a new branch with the same name in the remote repo. That means if topic-1 was previously pushed and a new branch topic-2 is copied from topic-1, "git push" on topic-2 branch won't push to the same branch as "git push on topic-1 branch would. 4. "git branch -c new-branch" should copy the currently checked out branch and create a new branch with name "new-branch". On Mon, May 29, 2017 at 4:09 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Sahil Dua <sahildua2305@xxxxxxxxx> writes: > > > New feature - copying a branch along with its config section. > > That's an unusual non-sentence (without a verb) in our commit message. Indeed terrible. I will remove it. > > > > Aim is to have an option -c for copying a branch just like -m option for > > renaming a branch. > > What should it copy literally, and what should it copy with > adjustment (and adjusting what), and what should it refrain from > copying? > > For example, when you create a branch topic-2 by copying from a > branch topic-1, do you copy the reflog in such a way that topic-2's > reflog contains all the entries of topic-1 before the copy happens, > capped with a new (and not shared with topic-1) entry that says > "Copied from topic-1"? When topic-1 is set to pull from a custom > upstream 'upstream' (i.e. not "origin")'s 'trunk' branch, by setting > 'branch.topic-2.remote' to "upstream", i.e. the same as the value of > 'branch.topic-1.remote' and 'branch.topic-2.merge' to "trunk", > i.e. the same as 'branch.topic-1.merge'? Should a "git push" while > topic-2 is checked out push to the same remote as a "git push" would > push to when topic-1 is checked out? Should they update the same > branch at the remote? Why and/or why not? [*1*] > > > This commit adds a few basic tests for getting any suggestions/feedback > > about expected behavior for this new feature. > > Writing tests is a good way to prepare for an implementation, which > must be done according to the design, but that happens after the > design is polished and judged to be a good one. While soliciting > comments on the design from others, tests are a bit too low level to > express what you are aiming at. It is a bit unhelpful to those who > may want to help to figure out answers to questions like the ones in > the previos paragraph (the one that begins with "For example,...") > by telling them to "read my intention from these tests", and when > you want as wide an input as possible, that is counterproductive for > your objective ;-). > > > Signed-off-by: Sahil Dua <sahildua2305@xxxxxxxxx> > > --- > > t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > > > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh > > index fe62e7c775da..2c95ed6ebf3c 100755 > > --- a/t/t3200-branch.sh > > +++ b/t/t3200-branch.sh > > @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' ' > > test_must_fail git config branch.s/s/dummy > > ' > > > > +test_expect_success 'git branch -c dumps usage' ' > > + test_expect_code 128 git branch -c 2>err && > > + test_i18ngrep "branch name required" err > > +' > > OK. Do we want to support a single-name format (i.e. copy from the > current)? > > > > > +git config branch.d.dummy Hello > > Write your tests to do as little outside test_expct_success block as > possible, and do a set-up close to where it matters. > Regarding putting code outside test_expect_success block, I wrote this to keep tests consistent within the same file as I saw other test doing it this way. How can I improve this? Here, I needed to set branch config before I copy so that I can confirm that the new "copied" branch has the same config as well. How can I improve this and take it as close to where-it-matters as possible? > > > +test_expect_success 'git branch -c d e should work' ' > > + git branch -l d && > > + git reflog exists refs/heads/d && > > + git branch -c d e && > > + git reflog exists refs/heads/d && > > + git reflog exists refs/heads/e > > +' > > A reasonable design of "copy" is for d's log to be left intact, and > e's log to begin with a single entry "created by copying from d". > Another possible design is to copy the log (making them identical), > and then add one entry to e's log "created by copying from d". > Yes, this is debatable as I mentioned above. My personal choice will be to use the second approach mentioned by you here. > > The above test would succeed with either implementation and does not > answer "should reflog be copied?" (see above about why "here are the > tests that shows my thinking--read them and comment" is a bad > approach). > > "move" that makes the source of the movement disappear after the > operation has a stronger justification of moving the log under the > new name (the only alternative is to lose the history of the source > forever), it is debatable if "copy" wants to retain a copy of the > history of an otherwise unrelated branch, which has its own history > retained after the operation. > For example, if branch topic-2 is copied from branch topic-1, "copy" option should make sure the history of branch topic-1 is retained. I will need suggestions about this too, in case I'm missing some consequences/corner-cases of this. > > > + > > +test_expect_success 'config information was copied, too' ' > > + test $(git config branch.e.dummy) = Hello && > > + test $(git config branch.d.dummy) = Hello > > +' > > The expectation is that a configuration like 'dummy' that does not > have any meaning to Git itself will all blindly be copied, which is > similar to what "move" does. Yes, any configuration for topic-1 branch should be copied to topic-2 branch while keeping the configuration for topic-1 branch retained. > > > > +git config branch.f/f.dummy Hello > > + > > +test_expect_success 'git branch -c f/f g/g should work' ' > > + git branch -l f/f && > > + git reflog exists refs/heads/f/f && > > + git branch -c f/f g/g && > > + git reflog exists refs/heads/f/f && > > + git reflog exists refs/heads/g/g > > +' > > Hmph. What's the reason to expect this not to work? > > > > > +test_expect_success 'config information was copied, too' ' > > + test $(git config branch.f/f.dummy) = Hello && > > + test $(git config branch.g/g.dummy) = Hello > > +' > > Isn't this part of the "should work" test above? The definition of > working is not just reflog is created for the new branch without the > old branch losing its reflog, right? > Right, I will put them in one test together. > > +test_expect_success 'git branch -c m2 m2 should work' ' > > + git branch -l m2 && > > + git reflog exists refs/heads/m2 && > > + git branch -c m2 m2 && > > + git reflog exists refs/heads/m2 > > +' > > Is it OK for the configuration be lost silently? No, I will change this accordingly as well. > > > Thanks. > > > [Footnote] > > *1* One way to avoid having to design an elaborate "definition of > copying" from scratch is to piggy-back on whatever "move" does. > Your definition of "copy" could be "copying from A to create B, > followed by deleting A, should leave the identical result as > moving A to create B". And ask people if they agree with that > definition as "the first principle". > > All the questions in the paragraph that begins with "For > example..." and similar ones can be answered by followingthe > first principle. > > Thanks.