On Thu, 18 Aug 2011, Jeff King wrote: > On Wed, Aug 17, 2011 at 09:56:11PM +0200, Michal Sojka wrote: > > > Hi, this is a proof of concept patch that allows editing of commits > > from gitk. I often review patches before pushing in gitk and if I > > would like to have an easy way of fixing typos in commit messages etc. > > > > So the patch adds "Edit this commit" item to gitk's context menu and > > the actual editing is done by non-interactively invoking interactive > > rebase :-) and git gui. > > Invoking rebase behind the scenes makes me very nervous. In particular: > > 1. There is nothing to indicate to the user that they are rewriting a > string of commits, which is going to wreak havoc if any of the > commits have been published elsewhere (either pushed somewhere, or > even present in another local branch). I.e., rebasing generally > needs to be a conscious decision of the user. I added a warning if the edited commit is contained in a remote branch. Would you consider this sufficient? > 2. Even if you accept the hazard of rewriting commits, you don't pass > "-p" to rebase. So it will linearize your history. I don't know how > robust "-p" is these days, and if it's up to the challenge of > people using it to rebase potentially large segments of complex > history. I added "-p" to rebase. I do not know about the robustness of "-p" as well, but is anything goes wrong during the rebase, git rebase --abort hopefully reverts everything. > So I think your idea is sane, and if you use it appropriately (by > editing commits in recent-ish linear stretches of history) your patch > works fine. But I really worry that it is going to be a problem for less > clueful users to stumble across in the menu. See the next version of the patch in a follow-up mail. -Michal -- 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