David Kastrup <dak@gnu.org> writes:The original was not even numbered so I cannot judge how the development proceeded, but I think the series is somewhat ill organized and the problem is not just commit log messages. For example, "vc-git.el: various improvements." removes NIL check for 'commit' and then later "Make several improvements" reinstates that check, perhaps because you realized the earlier change to remove it was a mistake (without the explanation in the log I can only guess the reason). Same for 'async to 0 change which is made by "Make synchronous for now". Preserving a honest, true-to-reality, history is one thing during the development, but when the overall improvements and enhancements reached a usable state, it is much more pleasant to review if the series is presented as if you knew what necessary changes are and what pitfalls to avoid from the beginning, and did your development in logical sequence, building one commit on top of the previous enhancements without "oops, that was a mistake so fix it while we are improving something else". My cursory review suggests that: * contrib/emacs/Makefile: Also install .el files. This is a good patch, independent from anything else. * contrib/emacs/vc-git.el: various improvements. * Make several improvements and get annotations to work (Emacs support pending). These two try to improve the same area, but the latter contains bugfixes for the former, not necessarily a logical succession (if it were a logical sequence to build and enhance, the first should be usable by itself, without a later bugfix). If these were to be made more than one logically independent patches, then probably the first would be to simplify vc-git-symbolic-commit implementation and update its callers (without having to say "oops, I needed the check for NIL commit after all" later), and without changing what the callers do. The second one would be "prev and next can be made much simpler". The third one would be "annotate can take 'rev' and extract-revision-at-line is now usable". But judging from how closely these changes are tied together (all of them depend on the external interface to vc-git-symbolic-commit) and how small each change is, I would probably make them a single commit that you do not have to say "oops" anywhere. * vc-git: support asynchronous annotations, and improve versioning. * (vc-git-annotate-command): Make synchronous for now. The same discussion here. The first one goes in the right direction, but then the moment of "oops" comes --- async does not necessarily work. Probably the right separation, if these two were to become two separate commits, would be to add async (but leave it synchronous, with a big code comment at the calling site of vc-do-command why you pass 0 instead of async), and then make "and improve versioning" part (whatever that is -- I cannot figure out which part of the patch "improve"-ment refers to) a separate commit. I would end this message with a very useful URL: http://article.gmane.org/gmane.comp.version-control.git/10894 I am with Linus 100%, regarding "honest" vs "disgusting" history. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
| Ingo Molnar | Re: [PATCH 00 of 36] x86/paravirt: groundwork for 64-bit Xen support |
| Linus Torvalds | Linux 2.6.27-rc8 |
| Alan Cox | [PATCH 02/27] drivers/char/hvc_console.c: adjust call to put_tty_driver |
| James Bottomley | Re: Integration of SCST in the mainstream Linux kernel |
git: | |
| Johannes Schindelin | RE: Switching from CVS to GIT |
| Florian v. Savigny | Can git be tweaked to work cross-platform, on FAT32? |
| Shawn Bohrer | [PATCH] Fix off by one error in prep_exclude. |
| Johannes Sixt | [PATCH 03/40] Add target architecture MinGW. |
| Marcos Laufer | dmesg IBM x3650 OpenBSD 4.3 |
| Nick Guenther | Re: Real men don't attack straw men |
| Steve B | Intel Atom and D945GCLF2 |
| Michael | QEMU /dev/tun issue with tun device number > 3 (more than 4 guests) |
| David Miller | [GIT]: Networking |
| Chuck Lever | Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin" |
| Patrick McHardy | gre: minor cleanups in netlink interface |
| Jarek Poplawski | Re: [PATCH] net_sched: Add qdisc __NET_XMIT_STOLEN flag |
