On Tue, 05 Dec 2006 17:13:20 -0800, Junio C Hamano wrote: > (This is offtopic) It's an interesting topic nonetheless, so I'll comment anyway. > (1) one commit exposes, then another fixes. > (2) one commit fixes, then another verifies the bug is no more. > (3) one commit to include both. I feel very strongly that I want (1) in the history. > unrelated bug. If the "test" is just an optional test in the > test suite, then it is easy to work around (the person who is > bisecting can ignore that bug by not running that particular > test), but if it is an assert somewhere deep inside the code, > ignoring it is not very easy, especially if the person who is > bisecting is not familiar with that part of the code. Granted, something like an assert that breaks the library would not be a useful thing to have in the history. I'm certainly not in favor of something like that. I'm talking about tests that demonstrate pre-existing broken-ness in the code. In the case of cairo, our test suite is entirely optional, and each test is out-of-process, so even if a test totally crashes the suite continues and it's easy to ignore that. But it's not just the correctness test suite. We also have a performance test suite, and I encourage the same "add test case, then performance fix" pattern there. This shows an even more obvious example of why it's useful to have separate commits in the history, (since someone may want to verify the performance impact on a separate system at any point in the future, and the two commits makes it easy to get "before" and "after" for the performance fix results from the new test case). > The sequence to split a patch in place would be (I'll speak in > the present tense and pretend Nico's "git add" does not exist > yet): > > git apply > git update-index <files for the first batch> > git commit > git commit -a ;# the remainder Yes, I can use this today, and I do, (as I mentioned in my mail). The only requirement is that I start with a non-dirty working tree. I can arrange that, but it would be just a bit less inconvenient if I didn't have to. > so you do not necessarily need a new "concept". No, I don't need it. And this "commit-index-content [paths...]" was the least significant part of my proposal. As I said, originally I was just going to say this "might be useful in some cases", but then someone just happened to request this feature on the list at the same time I was considering the proposal. Anyway, it spite of this being an accidental feature of y proposal, it seems to be the only part you commented on. Even if this functionality weren't made available at all, I'd still be interested in your comments on the main thrust of my proposal. I think that consists of: 1. Unifying the two current commands that provide commit-working-tree-content semantics into a single, use-oriented description. 2. Avoiding a change of semantics triggered by merely applying pathname arguments without any command-line option or alternate command name. > As I have already said (and you seemed to share the same > discipline), I do not like people committing anything > non-trivial that is not tested. Indeed. I like that discipline very much. And in fact, that's an important reason that I split a patch that I receive like this, (or just bounce it, which I often do, and would definitely do if it's not easy to split) > But with the way you said you want > to make the commits in the message I am responding to, the first > commit would never have been tested by anybody in isolation, not > by the original submitter even if he tested the patch before > giving it to you, nor you -- your working tree had either none > of his patch or all of it, and never was in the state with only > the first batch. Who said I wouldn't test it? I do split commits like this precisely so that I _can_ test it this way---and git helps a lot here. I do the split commit, then easily back up to the revision that adds the test case, verify the test fails before the bug fix, (which is something the maintainer doesn't get a chance to do with your (2) approach), then move forward and verify that the test passes after the fix. So, sure, I haven't ever had that working tree before the commit. But git makes it easy to get that working tree after I commit and test everything before I push anything out. [Incidentally, that's yet _another_ thing people can mention to friends who come from cvs and think that the separation of commit and push is annoying. And that's in addition to all of the performance advantages, the ability to work entirely offline, etc. etc.] > > The old behavior is still available with the --include option, but > > nobody has ever come out in favor of that being a useful command, > > That is a slight overstatement. OK. I should have worded that as "I wasn't aware of any arguments...". > That makes --include a less often used form. If > a merge is small and easy to resolve at only a few paths, it > still is handy to say "git commit -i resolved-path.c". It does > not add anything to the semantics -- it is only a typesaver. Oh, OK. I see it now. That's for combining the update-index, (or "resolve/resolved"---is there any consensus on that being a useful synonym for update-index here?) and the commit into a single command. I guess that's just a shortcut I've never used. So, yes, that shortcut would not fit cleanly into either of my proposed commit-working-tree-content nor commit-index-content. That would still require a two-stage: git update-index resolved-path.c git commit-index-content -Carl
Attachment:
pgpTXTSXhU3oO.pgp
Description: PGP signature