I originally had two series, but to retain my sanity while shuffling fixups, I based the entire batch on a rebase of jh/notes onto current master. The first four do not depend on jh/notes though, and are almost unchanged from the first round. The docs are probably sprinkled a bit strangely over the patches, not sure if it's worth a cleanup though (or I could just gather all the docs in a final patch). At least tests should be in the right place. Based on Hannes's comment: On Monday 15 February 2010 21:36:05 Johannes Sixt wrote: > Thomas Rast schrieb: > > +if test -x "$GIT_DIR"/hooks/post-rewrite && > > + test -s "$workdir"/../rewritten; then > > + "$GIT_DIR"/hooks/post-rewrite filter-branch < "$workdir"/../rewritten > > +fi > > This sounds extra-strange. As if filter-branch is used 20 times a day. If > the intent is to carry notes over to new commits, then filter-branch > should grow a --notes-filter instruction, no? With an easily accessible > copy-everything mode like --tagname-filter=cat. I made filter-branch extra-tweakable, but tried to stay consistent with the configuration of the rest of the series. Maybe it's a bit overengineered now, dunno. On Monday 15 February 2010 02:25:52 Johan Herland wrote: > On Sunday 14 February 2010, Thomas Rast wrote: > > > > I spent some time trying to refactor cmd_notes() into something > > that can nicely handle commands that do not fit the normal scheme, > > but eventually was too tired to continue and just crafted it in > > the existing code in the right place. [...] > As you say, cmd_notes() should be refactored, so that you don't have to > duplicate all of this (and some of the below stuff). Actually I meant something different: it took me quite some time to figure out what goes where in the flow of the current main() function, since it is sprinkled with if(subcommand) -- for most of them, several such clauses. I guess it made sense at the time, because all commands more or less worked with the same arguments and mostly the same background logic, just in a slightly different way. But I think it's quite unnatural for an entirely-different command like 'copy --stdin'. But maybe that's just me. > We (this goes for the existing commit_notes() call in cmd_notes() as well) > should either check the return value for commit_notes(), or (since it > currently only returns 0) we should change commit_notes() to void instead of > int. Actually, the correct solution is probably to "return error()" instead > of "die()" inside commit_notes(), and then check the return value. I'll leave that up to your judgement; I haven't looked into the API workings much, so I'm not sure when it should fail how. [Thanks for the other comments on my embarrassing mistakes; I was apparently rather too tired to continue programming.] Thomas Rast (11): Documentation: document post-rewrite hook commit --amend: invoke post-rewrite hook rebase: invoke post-rewrite hook rebase -i: invoke post-rewrite hook notes: clean up t3301 notes: implement 'git notes copy --stdin' notes: implement helpers needed for note copying during rewrite rebase: support automatic notes copying commit --amend: copy notes to the new commit filter-branch: invoke post-rewrite hook filter-branch: learn how to filter notes Documentation/config.txt | 17 +++ Documentation/git-filter-branch.txt | 20 +++- Documentation/git-notes.txt | 16 ++- Documentation/githooks.txt | 40 +++++ builtin-commit.c | 49 +++++++ builtin-notes.c | 134 +++++++++++++++++- builtin.h | 15 ++ git-am.sh | 13 ++ git-filter-branch.sh | 37 +++++ git-rebase--interactive.sh | 45 ++++++- git-rebase.sh | 6 + notes.c | 21 +++ notes.h | 9 ++ t/t3301-notes.sh | 273 +++++++++++++++++++++++------------ t/t3400-rebase.sh | 16 ++ t/t3404-rebase-interactive.sh | 10 ++ t/t5407-post-rewrite-hook.sh | 188 ++++++++++++++++++++++++ t/t7003-filter-branch.sh | 22 +++ t/t7501-commit.sh | 11 ++ 19 files changed, 843 insertions(+), 99 deletions(-) create mode 100755 t/t5407-post-rewrite-hook.sh -- 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