David Kastrup <dak@xxxxxxx> writes:
> How do I go about resubmitting with better comments? Make an extra
> branch and redo the part in artificial new commits?
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
