[Adding cc from elsewhere in this thread, for fairness.] Junio C Hamano venit, vidit, dixit 20.01.2010 22:08: > Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes: > >> Junio C Hamano venit, vidit, dixit 20.01.2010 21:10: >>> Joey Hess <joey@xxxxxxxxxxx> writes: >>> >>>> Do you think it makes sense for even git log --format=format:%s to be >>>> porcelain and potentially change when new features are used? >>> >>> If the series changed the meaning of "%s" format to mean "the subject of >>> the commit and notes information", with or without documenting it, then it >>> is just a bug we would like to fix. >> >> No, but outputting the note as part of the log is the standard. So for >> example, when you do a format-patch | apply cycle, format-patch will >> insert the note as part of the commit message, and apply will *store* >> the note text (including Note:\n) as part of the commit message of the >> new commit. > > Thanks; that was the kind of breakage report I was looking for (and wished > to have heard a lot earlier). Personally I find it is unexcusable that > format-patch defaults to giving notes. > >> So, I would say the notes feature is not that well integrated right now, > > No question about it. > >> I'm not complaining, I actually have this on a maybe-to-do list, but the >> way the series went kept me from investing time. > > Hmm, that hints there is a failure in the review and merge process. Care > to explain how we could have done better please? Well, I can only recall why it kept *me* from investing more time. I actually have very little free time available, I contribute to Git because it's fun, or I want a certain feature, or, admittedly, having a commit in a project like Git gives me a certain satisfaction. The notes feature looked very promising to me, and when Dscho came up with it (or followed up on someone else's proposal, I don't remember) I invested time in testing and contributing (minor) fixes. Then the series took on a life on it's own, first "disappearing" (when I was wondering if anything is going forward), then reappearing with (not only my) commits squashed in (which took away the satisfaction part), then going through a lengthy technical discussion on fan-out schemes (which was necessary, but took away the fun part). [The last two parts may have been the other way round, which doesn't matter.] When the first part of the series landed I began to look at it again and use it for the comments which go after the "--" part of a patch, only to find out that format-patch issue. I noticed the bad consequence on format-patch|apply only later. So I looked at the code, the log flags, and was about to code when the notes API changed again (on pu). So I decided to wait until the API is baked (that decision may have been influenced by my expectation that my patches would get squashed in again) and to fix that issue before 1.7.0. Note that I have to match the project's necessities with time availability on my side - otherwise I would have written that patch when more of that series had landed. Now I reported it because it came up in some disguise (and didn't want anyone spend time needlessly fixing a side issue), and I'm not the one fixing it, but that's fine. Besides the sociological aspect, I think you mentioned the main technical aspects: * If you introduce a new features, write extensive tests covering non-uses and mixed uses of the feature. * Write redundancy tests, such as checking that format-patch|apply and apply|format-patch both amount to the "identity" in the appropriate sense. Right now we do very atomic testing, which does have its merits (for determining the cause of a breakage). But since many features and commands are not orthogonal, atomic testing does not test for side effects, and test repos are minimal. Trying to test for specific combinations makes you miss some combinations, especially combinations with future features. But testing for those identity operations (quasi-noops, or cycles) should ensure some consistency. Maybe we should have a test repo which has all kinds of features turned on and used, and on which a set of those identities are tested. With every new feature, the repo as well as the set of supposed identities would need to be amended (maybe by cloning the repo, adding commits, and testing on an increasing set of repos). That would have caught at least the current issue immediately. Cheers, Michael -- 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