{"thread":{"id":"33959","subject":"[PATCH 0/3] cherry-pick: improvments","startedAt":"2013-05-28T12:59:04Z","lastAt":"2013-05-29T14:01:11Z","messageCount":18,"participants":["Felipe Contreras","Junio C Hamano","Thomas Rast","Ramkumar Ramachandra"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"218653","messageId":"1369745947-19416-1-git-send-email-felipe.contreras@gmail.com","threadId":"33959","inReplyTo":null,"subject":"[PATCH 0/3] cherry-pick: improvments","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-28T12:59:04Z","receivedAt":"2013-05-28T12:59:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere's a bunch of changes to make cherry-pick (and revert) more useful.\n\nFelipe Contreras (3):\n  cherry-pick: add support to copy notes\n  revert/cherry-pick: add --quiet option\n  revert/cherry-pick: add --skip option\n\n Documentation/git-cherry-pick.txt |   7 +-\n Documentation/git-revert.txt      |   7 +-\n Documentation/sequencer.txt       |   3 +\n builtin/revert.c                  |   9 +++\n sequencer.c                       | 162 +++++++++++++++++++++++++++++++++++++-\n sequencer.h                       |   6 +-\n t/t3500-cherry.sh                 |  32 ++++++++\n t/t3510-cherry-pick-sequence.sh   |  12 +++\n 8 files changed, 232 insertions(+), 6 deletions(-)\n\n-- \n1.8.3.rc3.312.g47657de\n"},{"id":"218654","messageId":"1369745947-19416-2-git-send-email-felipe.contreras@gmail.com","threadId":"33959","inReplyTo":"1369745947-19416-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-28T12:59:05Z","receivedAt":"2013-05-28T12:59:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/revert.c  |   2 +\n sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n sequencer.h       |   2 +\n t/t3500-cherry.sh |  32 +++++++++++++\n 4 files changed, 169 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 0e5ce71..b977124 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -119,6 +119,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n+\t\tOPT_END(),\n \t};\n \n \tif (opts->action == REPLAY_PICK) {\n@@ -129,6 +130,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tOPT_BOOLEAN(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n \t\t\tOPT_BOOLEAN(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n \t\t\tOPT_BOOLEAN(0, \"skip-empty\", &opts->skip_empty, N_(\"skip empty commits\")),\n+\t\t\tOPT_BOOLEAN(0, \"copy-notes\", &opts->copy_notes, N_(\"copy notes\")),\n \t\t\tOPT_END(),\n \t\t};\n \t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\ndiff --git a/sequencer.c b/sequencer.c\nindex edf141d..35a84ad 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -20,6 +20,18 @@\n const char sign_off_header[] = \"Signed-off-by: \";\n static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n \n+struct rewritten_list_item {\n+\tunsigned char from[20];\n+\tunsigned char to[20];\n+};\n+\n+struct rewritten_list {\n+\tstruct rewritten_list_item *items;\n+\tunsigned int nr, alloc;\n+};\n+\n+static struct rewritten_list rewritten;\n+\n static int is_rfc2822_line(const char *buf, int len)\n {\n \tint i;\n@@ -102,6 +114,40 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,\n \treturn 1;\n }\n \n+static void add_rewritten(unsigned char *from, unsigned char *to)\n+{\n+\tstruct rewritten_list_item *item;\n+\tif (rewritten.nr + 1 >= rewritten.alloc) {\n+\t\trewritten.alloc += 32;\n+\t\trewritten.items = xrealloc(rewritten.items, rewritten.alloc * sizeof(*rewritten.items));\n+\t}\n+\titem = &rewritten.items[rewritten.nr];\n+\thashcpy(item->from, from);\n+\thashcpy(item->to, to);\n+\trewritten.nr++;\n+}\n+\n+static void copy_notes(void)\n+{\n+\tunsigned char note_commit[20];\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct notes_tree *t = &default_notes_tree;\n+\tint i;\n+\n+\tinit_notes(t, NULL, NULL, 0);\n+\n+\tfor (i = 0; i < rewritten.nr; i++) {\n+\t\tstruct rewritten_list_item *item = &rewritten.items[i];\n+\t\tcopy_note(t, item->from, item->to, 0, NULL);\n+\t}\n+\n+\tstrbuf_addstr(&buf, \"Notes added by 'git cherry-pick'\\n\");\n+\tcreate_notes_commit(&default_notes_tree, NULL, &buf, note_commit);\n+\tstrbuf_insert(&buf, 0, \"notes: \", 7);\n+\tupdate_ref(buf.buf, t->ref, note_commit, NULL, 0, MSG_ON_ERR);\n+\tstrbuf_release(&buf);\n+}\n+\n static void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -641,6 +687,14 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t\tres = run_git_commit(defmsg, opts, allow);\n \t}\n \n+\tif (!res && opts->copy_notes) {\n+\t\tunsigned char to[20];\n+\n+\t\tif (read_ref(\"HEAD\", to))\n+\t\t\tgoto leave;\n+\n+\t\tadd_rewritten(commit->object.sha1, to);\n+\t}\n leave:\n \tfree_message(&msg);\n \tfree(defmsg);\n@@ -786,6 +840,40 @@ static void read_populate_todo(struct commit_list **todo_list,\n \t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n }\n \n+static void read_populate_rewritten(void)\n+{\n+\tconst char *rewritten_file = git_path(SEQ_REWR_FILE);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *p;\n+\tint fd;\n+\n+\tfd = open(rewritten_file, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn;\n+\tif (strbuf_read(&buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\tstrbuf_release(&buf);\n+\t\treturn;\n+\t}\n+\tclose(fd);\n+\n+\tfor (p = buf.buf; *p;) {\n+\t\tunsigned char from[20];\n+\t\tunsigned char to[20];\n+\t\tchar *eol = strchrnul(p, '\\n');\n+\t\tif (eol - p != 81)\n+\t\t\t/* wrong size */\n+\t\t\tbreak;\n+\t\tif (get_sha1_hex(p, from))\n+\t\t\tbreak;\n+\t\tif (get_sha1_hex(p + 41, to))\n+\t\t\tbreak;\n+\t\tadd_rewritten(from, to);\n+\t\tp = *eol ? eol + 1 : eol;\n+\t}\n+\tstrbuf_release(&buf);\n+}\n+\n static int populate_opts_cb(const char *key, const char *value, void *data)\n {\n \tstruct replay_opts *opts = data;\n@@ -810,7 +898,10 @@ static int populate_opts_cb(const char *key, const char *value, void *data)\n \telse if (!strcmp(key, \"options.strategy-option\")) {\n \t\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n \t\topts->xopts[opts->xopts_nr++] = xstrdup(value);\n-\t} else\n+\t}\n+\telse if (!strcmp(key, \"options.copy-notes\"))\n+\t\topts->copy_notes = git_config_bool_or_int(key, value, &error_flag);\n+\telse\n \t\treturn error(_(\"Invalid key: %s\"), key);\n \n \tif (!error_flag)\n@@ -958,6 +1049,29 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n \tstrbuf_release(&buf);\n }\n \n+static void save_rewritten(void)\n+{\n+\tconst char *todo_file = git_path(SEQ_REWR_FILE);\n+\tstatic struct lock_file rewritten_lock;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint fd, i;\n+\n+\tfd = hold_lock_file_for_update(&rewritten_lock, todo_file, LOCK_DIE_ON_ERROR);\n+\tfor (i = 0; i < rewritten.nr; i++) {\n+\t\tstruct rewritten_list_item *item = &rewritten.items[i];\n+\t\tstrbuf_addf(&buf, \"%s %s\\n\", sha1_to_hex(item->from), sha1_to_hex(item->to));\n+\t}\n+\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n+\t\tstrbuf_release(&buf);\n+\t\tdie_errno(_(\"Could not write to %s\"), todo_file);\n+\t}\n+\tif (commit_lock_file(&rewritten_lock) < 0) {\n+\t\tstrbuf_release(&buf);\n+\t\tdie(_(\"Error wrapping up %s.\"), todo_file);\n+\t}\n+\tstrbuf_release(&buf);\n+}\n+\n static void save_opts(struct replay_opts *opts)\n {\n \tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n@@ -987,6 +1101,8 @@ static void save_opts(struct replay_opts *opts)\n \t\t\t\t\t\t\t\"options.strategy-option\",\n \t\t\t\t\t\t\topts->xopts[i], \"^$\", 0);\n \t}\n+\tif (opts->copy_notes)\n+\t\tgit_config_set_in_file(opts_file, \"options.copy-notes\", \"true\");\n }\n \n static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n@@ -1003,10 +1119,16 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \tfor (cur = todo_list; cur; cur = cur->next) {\n \t\tsave_todo(cur, opts);\n \t\tres = do_pick_commit(cur->item, opts);\n-\t\tif (res)\n+\t\tif (res) {\n+\t\t\tif (opts->copy_notes)\n+\t\t\t\tsave_rewritten();\n \t\t\treturn res;\n+\t\t}\n \t}\n \n+\tif (opts->copy_notes)\n+\t\tcopy_notes();\n+\n \t/*\n \t * Sequence of picks finished successfully; cleanup by\n \t * removing the .git/sequencer directory\n@@ -1033,6 +1155,8 @@ static int sequencer_continue(struct replay_opts *opts)\n \t\treturn continue_single_pick();\n \tread_populate_opts(&opts);\n \tread_populate_todo(&todo_list, opts);\n+\tif (opts->copy_notes)\n+\t\tread_populate_rewritten();\n \n \t/* Verify that the conflict has been resolved */\n \tif (file_exists(git_path(\"CHERRY_PICK_HEAD\")) ||\n@@ -1049,8 +1173,14 @@ static int sequencer_continue(struct replay_opts *opts)\n \n static int single_pick(struct commit *cmit, struct replay_opts *opts)\n {\n+\tint ret;\n \tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n-\treturn do_pick_commit(cmit, opts);\n+\tret = do_pick_commit(cmit, opts);\n+\tif (ret)\n+\t\treturn ret;\n+\tif (opts->copy_notes)\n+\t\tcopy_notes();\n+\treturn 0;\n }\n \n int sequencer_pick_revisions(struct replay_opts *opts)\ndiff --git a/sequencer.h b/sequencer.h\nindex 3b04844..6cc072c 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -5,6 +5,7 @@\n #define SEQ_HEAD_FILE\t\"sequencer/head\"\n #define SEQ_TODO_FILE\t\"sequencer/todo\"\n #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n+#define SEQ_REWR_FILE\t\"sequencer/rewritten\"\n \n #define APPEND_SIGNOFF_DEDUP (1u << 0)\n \n@@ -35,6 +36,7 @@ struct replay_opts {\n \tint allow_empty_message;\n \tint keep_redundant_commits;\n \tint skip_empty;\n+\tint copy_notes;\n \n \tint mainline;\n \ndiff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\nindex f038f34..79c1219 100755\n--- a/t/t3500-cherry.sh\n+++ b/t/t3500-cherry.sh\n@@ -55,4 +55,36 @@ test_expect_success \\\n      expr \"$(echo $(git cherry master my-topic-branch) )\" : \"+ [^ ]* - .*\"\n '\n \n+test_expect_success \\\n+    'copy notes' \\\n+    'git checkout master &&\n+    echo notes > C &&\n+    test_tick &&\n+    git commit -a -m \"Update C\" &&\n+    git notes add -m \"a note\" &&\n+    git checkout -b note-test HEAD^ &&\n+    git cherry-pick --copy-notes -x master &&\n+    test \"a note\" = \"$(git notes show HEAD)\"\n+'\n+\n+test_expect_success \\\n+    'copy multiple notes' \\\n+    'git checkout master &&\n+    echo \"multiple notes\" > C &&\n+    git commit -a -m \"Update C again\" &&\n+    git notes add -m \"another note\" &&\n+    git commit -a -m \"Empty\" --allow-empty &&\n+    echo \"more notes\" > C &&\n+    git commit -a -m \"Update C once more\" &&\n+    git notes add -m \"final note\" &&\n+    git checkout note-test &&\n+    git reset --hard master~3 &&\n+    test_expect_code 1 git cherry-pick --copy-notes -x HEAD..master &&\n+    git reset --hard &&\n+    git cherry-pick --continue\n+    test \"a note\" = \"$(git notes show HEAD~2)\" &&\n+    test \"another note\" = \"$(git notes show HEAD~1)\" &&\n+    test \"final note\" = \"$(git notes show HEAD)\"\n+'\n+\n test_done\n-- \n1.8.3.rc3.312.g47657de\n"},{"id":"218655","messageId":"1369745947-19416-3-git-send-email-felipe.contreras@gmail.com","threadId":"33959","inReplyTo":"1369745947-19416-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/3] revert/cherry-pick: add --quiet option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-28T12:59:06Z","receivedAt":"2013-05-28T12:59:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-cherry-pick.txt | 6 +++++-\n Documentation/git-revert.txt      | 6 +++++-\n builtin/revert.c                  | 1 +\n sequencer.c                       | 2 ++\n sequencer.h                       | 1 +\n 5 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex fccd936..da0bd81 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -8,7 +8,7 @@ git-cherry-pick - Apply the changes introduced by some existing commits\n SYNOPSIS\n --------\n [verse]\n-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...\n+'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...\n 'git cherry-pick' --continue\n 'git cherry-pick' --quit\n 'git cherry-pick' --abort\n@@ -51,6 +51,10 @@ OPTIONS\n \tfeed all <commit>... arguments to a single revision walk\n \t(see a later example that uses 'maint master..next').\n \n+-q::\n+--quiet::\n+\tQuiet, suppress feedback messages.\n+\n -e::\n --edit::\n \tWith this option, 'git cherry-pick' will let you edit the commit\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex f79c9d8..98a8e7a 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -8,7 +8,7 @@ git-revert - Revert some existing commits\n SYNOPSIS\n --------\n [verse]\n-'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...\n+'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...\n 'git revert' --continue\n 'git revert' --quit\n 'git revert' --abort\n@@ -40,6 +40,10 @@ OPTIONS\n \tdefault, see linkgit:git-rev-list[1] and its '--no-walk'\n \toption.\n \n+-q::\n+--quiet::\n+\tQuiet, suppress feedback messages.\n+\n -e::\n --edit::\n \tWith this option, 'git revert' will let you edit the commit\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex b977124..d63b4a6 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -100,6 +100,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tint contin = 0;\n \tint rollback = 0;\n \tstruct option options[] = {\n+\t\tOPT__QUIET(&opts->quiet, N_(\"suppress progress reporting\")),\n \t\tOPT_BOOLEAN(0, \"quit\", &remove_state, N_(\"end revert or cherry-pick sequence\")),\n \t\tOPT_BOOLEAN(0, \"continue\", &contin, N_(\"resume revert or cherry-pick sequence\")),\n \t\tOPT_BOOLEAN(0, \"abort\", &rollback, N_(\"cancel revert or cherry-pick sequence\")),\ndiff --git a/sequencer.c b/sequencer.c\nindex 35a84ad..b4e395a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -435,6 +435,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \targv_array_init(&array);\n \targv_array_push(&array, \"commit\");\n \targv_array_push(&array, \"-n\");\n+\tif (opts->quiet)\n+\t\targv_array_push(&array, \"-q\");\n \n \tif (opts->signoff)\n \t\targv_array_push(&array, \"-s\");\ndiff --git a/sequencer.h b/sequencer.h\nindex 6cc072c..41e19d0 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -37,6 +37,7 @@ struct replay_opts {\n \tint keep_redundant_commits;\n \tint skip_empty;\n \tint copy_notes;\n+\tint quiet;\n \n \tint mainline;\n \n-- \n1.8.3.rc3.312.g47657de\n"},{"id":"218656","messageId":"1369745947-19416-4-git-send-email-felipe.contreras@gmail.com","threadId":"33959","inReplyTo":"1369745947-19416-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/3] revert/cherry-pick: add --skip option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-28T12:59:07Z","receivedAt":"2013-05-28T12:59:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Akin to 'am --skip' and 'rebase --skip'.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-cherry-pick.txt |  1 +\n Documentation/git-revert.txt      |  1 +\n Documentation/sequencer.txt       |  3 +++\n builtin/revert.c                  |  6 ++++++\n sequencer.c                       | 24 ++++++++++++++++++++++++\n sequencer.h                       |  3 ++-\n t/t3510-cherry-pick-sequence.sh   | 12 ++++++++++++\n 7 files changed, 49 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex da0bd81..d95c63c 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...\n 'git cherry-pick' --continue\n+'git cherry-pick' --skip\n 'git cherry-pick' --quit\n 'git cherry-pick' --abort\n \ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 98a8e7a..52e146e 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...\n 'git revert' --continue\n+'git revert' --skip\n 'git revert' --quit\n 'git revert' --abort\n \ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 5747f44..df2d355 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -3,6 +3,9 @@\n \t'.git/sequencer'.  Can be used to continue after resolving\n \tconflicts in a failed cherry-pick or revert.\n \n+--skip::\n+\tSkip the current commit, and then continue.\n+\n --quit::\n \tForget about the current operation in progress.  Can be used\n \tto clear the sequencer state after a failed cherry-pick or\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex d63b4a6..6afd990 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tint remove_state = 0;\n \tint contin = 0;\n \tint rollback = 0;\n+\tint skip = 0;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&opts->quiet, N_(\"suppress progress reporting\")),\n \t\tOPT_BOOLEAN(0, \"quit\", &remove_state, N_(\"end revert or cherry-pick sequence\")),\n \t\tOPT_BOOLEAN(0, \"continue\", &contin, N_(\"resume revert or cherry-pick sequence\")),\n \t\tOPT_BOOLEAN(0, \"abort\", &rollback, N_(\"cancel revert or cherry-pick sequence\")),\n+\t\tOPT_BOOLEAN(0, \"skip\", &skip, N_(\"skip current commit in the sequence\")),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n \t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -160,6 +162,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->subcommand = REPLAY_CONTINUE;\n \telse if (rollback)\n \t\topts->subcommand = REPLAY_ROLLBACK;\n+\telse if (skip)\n+\t\topts->subcommand = REPLAY_SKIP;\n \telse\n \t\topts->subcommand = REPLAY_NONE;\n \n@@ -170,6 +174,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tthis_operation = \"--quit\";\n \t\telse if (opts->subcommand == REPLAY_CONTINUE)\n \t\t\tthis_operation = \"--continue\";\n+\t\telse if (opts->subcommand == REPLAY_SKIP)\n+\t\t\tthis_operation = \"--skip\";\n \t\telse {\n \t\t\tassert(opts->subcommand == REPLAY_ROLLBACK);\n \t\t\tthis_operation = \"--abort\";\ndiff --git a/sequencer.c b/sequencer.c\nindex b4e395a..971cab2 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1173,6 +1173,28 @@ static int sequencer_continue(struct replay_opts *opts)\n \treturn pick_commits(todo_list, opts);\n }\n \n+static int sequencer_skip(struct replay_opts *opts)\n+{\n+\tconst char *argv[4]; /* reset --hard HEAD + NULL */\n+\tstruct string_list merge_rr = STRING_LIST_INIT_DUP;\n+\tint ret;\n+\n+\tif (setup_rerere(&merge_rr, 0) >= 0) {\n+\t\trerere_clear(&merge_rr);\n+\t\tstring_list_clear(&merge_rr, 1);\n+\t}\n+\n+\targv[0] = \"reset\";\n+\targv[1] = \"--hard\";\n+\targv[2] = \"HEAD\";\n+\targv[3] = NULL;\n+\tret = run_command_v_opt(argv, RUN_GIT_CMD);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\treturn sequencer_continue(opts);\n+}\n+\n static int single_pick(struct commit *cmit, struct replay_opts *opts)\n {\n \tint ret;\n@@ -1209,6 +1231,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \t\treturn sequencer_rollback(opts);\n \tif (opts->subcommand == REPLAY_CONTINUE)\n \t\treturn sequencer_continue(opts);\n+\tif (opts->subcommand == REPLAY_SKIP)\n+\t\treturn sequencer_skip(opts);\n \n \tfor (i = 0; i < opts->revs->pending.nr; i++) {\n \t\tunsigned char sha1[20];\ndiff --git a/sequencer.h b/sequencer.h\nindex 41e19d0..b9e09ac 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -18,7 +18,8 @@ enum replay_subcommand {\n \tREPLAY_NONE,\n \tREPLAY_REMOVE_STATE,\n \tREPLAY_CONTINUE,\n-\tREPLAY_ROLLBACK\n+\tREPLAY_ROLLBACK,\n+\tREPLAY_SKIP\n };\n \n struct replay_opts {\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 7b7a89d..f58a83d 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -511,4 +511,16 @@ test_expect_success 'commit descriptions in insn sheet are optional' '\n \ttest_line_count = 4 commits\n '\n \n+test_expect_success 'skip' '\n+\tpristine_detach conflicting &&\n+\ttest_must_fail git cherry-pick initial..picked &&\n+\n+\tgit checkout HEAD -- unrelated &&\n+\ttest_must_fail git cherry-pick --continue &&\n+\tgit cherry-pick --skip &&\n+\n+\tgit rev-list initial..HEAD >commits &&\n+\ttest_line_count = 3 commits\n+'\n+\n test_done\n-- \n1.8.3.rc3.312.g47657de\n"},{"id":"218681","messageId":"7vobbv119k.fsf@alter.siamese.dyndns.org","threadId":"33959","inReplyTo":"1369745947-19416-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-28T17:07:35Z","receivedAt":"2013-05-28T17:07:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  builtin/revert.c  |   2 +\n>  sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n>  sequencer.h       |   2 +\n>  t/t3500-cherry.sh |  32 +++++++++++++\n>  4 files changed, 169 insertions(+), 3 deletions(-)\n\n\"git cherry-pick\" should help maintaining notes just like amend and\nrebase, but how should this interact with notes.rewrite.<command>,\nwhere the command is capable of doing this without an explicit\noption once you tell which notes need to be maintained?\n\nThomas Rast Cc'ed as he has been the primary force behind this line\nof \"notes\" usability.\n\nIt probably is not sensible to carry over note from a reverted\ncommit to its revert, but I didn't immediately spot how the patch\ndoes this only for cherry-pick but not for revert (the codepath in\ndo_pick_commit() is shared between them, no?).\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 0e5ce71..b977124 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -119,6 +119,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>  \t\tOPT_END(),\n>  \t\tOPT_END(),\n>  \t\tOPT_END(),\n> +\t\tOPT_END(),\n>  \t};\n>  \n>  \tif (opts->action == REPLAY_PICK) {\n> @@ -129,6 +130,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>  \t\t\tOPT_BOOLEAN(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n>  \t\t\tOPT_BOOLEAN(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n>  \t\t\tOPT_BOOLEAN(0, \"skip-empty\", &opts->skip_empty, N_(\"skip empty commits\")),\n> +\t\t\tOPT_BOOLEAN(0, \"copy-notes\", &opts->copy_notes, N_(\"copy notes\")),\n>  \t\t\tOPT_END(),\n>  \t\t};\n>  \t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\n> diff --git a/sequencer.c b/sequencer.c\n> index edf141d..35a84ad 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -20,6 +20,18 @@\n>  const char sign_off_header[] = \"Signed-off-by: \";\n>  static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n>  \n> +struct rewritten_list_item {\n> +\tunsigned char from[20];\n> +\tunsigned char to[20];\n> +};\n> +\n> +struct rewritten_list {\n> +\tstruct rewritten_list_item *items;\n> +\tunsigned int nr, alloc;\n> +};\n> +\n> +static struct rewritten_list rewritten;\n> +\n>  static int is_rfc2822_line(const char *buf, int len)\n>  {\n>  \tint i;\n> @@ -102,6 +114,40 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,\n>  \treturn 1;\n>  }\n>  \n> +static void add_rewritten(unsigned char *from, unsigned char *to)\n> +{\n> +\tstruct rewritten_list_item *item;\n> +\tif (rewritten.nr + 1 >= rewritten.alloc) {\n> +\t\trewritten.alloc += 32;\n> +\t\trewritten.items = xrealloc(rewritten.items, rewritten.alloc * sizeof(*rewritten.items));\n> +\t}\n> +\titem = &rewritten.items[rewritten.nr];\n> +\thashcpy(item->from, from);\n> +\thashcpy(item->to, to);\n> +\trewritten.nr++;\n> +}\n> +\n> +static void copy_notes(void)\n> +{\n> +\tunsigned char note_commit[20];\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct notes_tree *t = &default_notes_tree;\n> +\tint i;\n> +\n> +\tinit_notes(t, NULL, NULL, 0);\n> +\n> +\tfor (i = 0; i < rewritten.nr; i++) {\n> +\t\tstruct rewritten_list_item *item = &rewritten.items[i];\n> +\t\tcopy_note(t, item->from, item->to, 0, NULL);\n> +\t}\n> +\n> +\tstrbuf_addstr(&buf, \"Notes added by 'git cherry-pick'\\n\");\n> +\tcreate_notes_commit(&default_notes_tree, NULL, &buf, note_commit);\n> +\tstrbuf_insert(&buf, 0, \"notes: \", 7);\n> +\tupdate_ref(buf.buf, t->ref, note_commit, NULL, 0, MSG_ON_ERR);\n> +\tstrbuf_release(&buf);\n> +}\n> +\n>  static void remove_sequencer_state(void)\n>  {\n>  \tstruct strbuf seq_dir = STRBUF_INIT;\n> @@ -641,6 +687,14 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>  \t\t\tres = run_git_commit(defmsg, opts, allow);\n>  \t}\n>  \n> +\tif (!res && opts->copy_notes) {\n> +\t\tunsigned char to[20];\n> +\n> +\t\tif (read_ref(\"HEAD\", to))\n> +\t\t\tgoto leave;\n> +\n> +\t\tadd_rewritten(commit->object.sha1, to);\n> +\t}\n>  leave:\n>  \tfree_message(&msg);\n>  \tfree(defmsg);\n> @@ -786,6 +840,40 @@ static void read_populate_todo(struct commit_list **todo_list,\n>  \t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n>  }\n>  \n> +static void read_populate_rewritten(void)\n> +{\n> +\tconst char *rewritten_file = git_path(SEQ_REWR_FILE);\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tchar *p;\n> +\tint fd;\n> +\n> +\tfd = open(rewritten_file, O_RDONLY);\n> +\tif (fd < 0)\n> +\t\treturn;\n> +\tif (strbuf_read(&buf, fd, 0) < 0) {\n> +\t\tclose(fd);\n> +\t\tstrbuf_release(&buf);\n> +\t\treturn;\n> +\t}\n> +\tclose(fd);\n> +\n> +\tfor (p = buf.buf; *p;) {\n> +\t\tunsigned char from[20];\n> +\t\tunsigned char to[20];\n> +\t\tchar *eol = strchrnul(p, '\\n');\n> +\t\tif (eol - p != 81)\n> +\t\t\t/* wrong size */\n> +\t\t\tbreak;\n> +\t\tif (get_sha1_hex(p, from))\n> +\t\t\tbreak;\n> +\t\tif (get_sha1_hex(p + 41, to))\n> +\t\t\tbreak;\n> +\t\tadd_rewritten(from, to);\n> +\t\tp = *eol ? eol + 1 : eol;\n> +\t}\n> +\tstrbuf_release(&buf);\n> +}\n> +\n>  static int populate_opts_cb(const char *key, const char *value, void *data)\n>  {\n>  \tstruct replay_opts *opts = data;\n> @@ -810,7 +898,10 @@ static int populate_opts_cb(const char *key, const char *value, void *data)\n>  \telse if (!strcmp(key, \"options.strategy-option\")) {\n>  \t\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n>  \t\topts->xopts[opts->xopts_nr++] = xstrdup(value);\n> -\t} else\n> +\t}\n> +\telse if (!strcmp(key, \"options.copy-notes\"))\n> +\t\topts->copy_notes = git_config_bool_or_int(key, value, &error_flag);\n> +\telse\n>  \t\treturn error(_(\"Invalid key: %s\"), key);\n>  \n>  \tif (!error_flag)\n> @@ -958,6 +1049,29 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n>  \tstrbuf_release(&buf);\n>  }\n>  \n> +static void save_rewritten(void)\n> +{\n> +\tconst char *todo_file = git_path(SEQ_REWR_FILE);\n> +\tstatic struct lock_file rewritten_lock;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tint fd, i;\n> +\n> +\tfd = hold_lock_file_for_update(&rewritten_lock, todo_file, LOCK_DIE_ON_ERROR);\n> +\tfor (i = 0; i < rewritten.nr; i++) {\n> +\t\tstruct rewritten_list_item *item = &rewritten.items[i];\n> +\t\tstrbuf_addf(&buf, \"%s %s\\n\", sha1_to_hex(item->from), sha1_to_hex(item->to));\n> +\t}\n> +\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n> +\t\tstrbuf_release(&buf);\n> +\t\tdie_errno(_(\"Could not write to %s\"), todo_file);\n> +\t}\n> +\tif (commit_lock_file(&rewritten_lock) < 0) {\n> +\t\tstrbuf_release(&buf);\n> +\t\tdie(_(\"Error wrapping up %s.\"), todo_file);\n> +\t}\n> +\tstrbuf_release(&buf);\n> +}\n> +\n>  static void save_opts(struct replay_opts *opts)\n>  {\n>  \tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n> @@ -987,6 +1101,8 @@ static void save_opts(struct replay_opts *opts)\n>  \t\t\t\t\t\t\t\"options.strategy-option\",\n>  \t\t\t\t\t\t\topts->xopts[i], \"^$\", 0);\n>  \t}\n> +\tif (opts->copy_notes)\n> +\t\tgit_config_set_in_file(opts_file, \"options.copy-notes\", \"true\");\n>  }\n>  \n>  static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n> @@ -1003,10 +1119,16 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n>  \tfor (cur = todo_list; cur; cur = cur->next) {\n>  \t\tsave_todo(cur, opts);\n>  \t\tres = do_pick_commit(cur->item, opts);\n> -\t\tif (res)\n> +\t\tif (res) {\n> +\t\t\tif (opts->copy_notes)\n> +\t\t\t\tsave_rewritten();\n>  \t\t\treturn res;\n> +\t\t}\n>  \t}\n>  \n> +\tif (opts->copy_notes)\n> +\t\tcopy_notes();\n> +\n>  \t/*\n>  \t * Sequence of picks finished successfully; cleanup by\n>  \t * removing the .git/sequencer directory\n> @@ -1033,6 +1155,8 @@ static int sequencer_continue(struct replay_opts *opts)\n>  \t\treturn continue_single_pick();\n>  \tread_populate_opts(&opts);\n>  \tread_populate_todo(&todo_list, opts);\n> +\tif (opts->copy_notes)\n> +\t\tread_populate_rewritten();\n>  \n>  \t/* Verify that the conflict has been resolved */\n>  \tif (file_exists(git_path(\"CHERRY_PICK_HEAD\")) ||\n> @@ -1049,8 +1173,14 @@ static int sequencer_continue(struct replay_opts *opts)\n>  \n>  static int single_pick(struct commit *cmit, struct replay_opts *opts)\n>  {\n> +\tint ret;\n>  \tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n> -\treturn do_pick_commit(cmit, opts);\n> +\tret = do_pick_commit(cmit, opts);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tif (opts->copy_notes)\n> +\t\tcopy_notes();\n> +\treturn 0;\n>  }\n>  \n>  int sequencer_pick_revisions(struct replay_opts *opts)\n> diff --git a/sequencer.h b/sequencer.h\n> index 3b04844..6cc072c 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -5,6 +5,7 @@\n>  #define SEQ_HEAD_FILE\t\"sequencer/head\"\n>  #define SEQ_TODO_FILE\t\"sequencer/todo\"\n>  #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n> +#define SEQ_REWR_FILE\t\"sequencer/rewritten\"\n>  \n>  #define APPEND_SIGNOFF_DEDUP (1u << 0)\n>  \n> @@ -35,6 +36,7 @@ struct replay_opts {\n>  \tint allow_empty_message;\n>  \tint keep_redundant_commits;\n>  \tint skip_empty;\n> +\tint copy_notes;\n>  \n>  \tint mainline;\n>  \n> diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\n> index f038f34..79c1219 100755\n> --- a/t/t3500-cherry.sh\n> +++ b/t/t3500-cherry.sh\n> @@ -55,4 +55,36 @@ test_expect_success \\\n>       expr \"$(echo $(git cherry master my-topic-branch) )\" : \"+ [^ ]* - .*\"\n>  '\n>  \n> +test_expect_success \\\n> +    'copy notes' \\\n> +    'git checkout master &&\n> +    echo notes > C &&\n> +    test_tick &&\n> +    git commit -a -m \"Update C\" &&\n> +    git notes add -m \"a note\" &&\n> +    git checkout -b note-test HEAD^ &&\n> +    git cherry-pick --copy-notes -x master &&\n> +    test \"a note\" = \"$(git notes show HEAD)\"\n> +'\n> +\n> +test_expect_success \\\n> +    'copy multiple notes' \\\n> +    'git checkout master &&\n> +    echo \"multiple notes\" > C &&\n> +    git commit -a -m \"Update C again\" &&\n> +    git notes add -m \"another note\" &&\n> +    git commit -a -m \"Empty\" --allow-empty &&\n> +    echo \"more notes\" > C &&\n> +    git commit -a -m \"Update C once more\" &&\n> +    git notes add -m \"final note\" &&\n> +    git checkout note-test &&\n> +    git reset --hard master~3 &&\n> +    test_expect_code 1 git cherry-pick --copy-notes -x HEAD..master &&\n> +    git reset --hard &&\n> +    git cherry-pick --continue\n> +    test \"a note\" = \"$(git notes show HEAD~2)\" &&\n> +    test \"another note\" = \"$(git notes show HEAD~1)\" &&\n> +    test \"final note\" = \"$(git notes show HEAD)\"\n> +'\n> +\n>  test_done\n"},{"id":"218689","messageId":"87mwrfx9ug.fsf@linux-k42r.v.cablecom.net","threadId":"33959","inReplyTo":"7vobbv119k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-28T18:01:11Z","receivedAt":"2013-05-28T18:01:11Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Thomas Rast Cc'ed as he has been the primary force behind this line\n> of \"notes\" usability.\n\nThanks for pointing this out to me.\n\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  builtin/revert.c  |   2 +\n>>  sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n>>  sequencer.h       |   2 +\n>>  t/t3500-cherry.sh |  32 +++++++++++++\n>>  4 files changed, 169 insertions(+), 3 deletions(-)\n>\n> \"git cherry-pick\" should help maintaining notes just like amend and\n> rebase, but how should this interact with notes.rewrite.<command>,\n> where the command is capable of doing this without an explicit\n> option once you tell which notes need to be maintained?\n\nSince we already have the notes.rewrite.<command> convention, it would\nseem the obvious choice to line it up with the others.  The main\nbikeshedding opportunity is whether this should be an exception and\ndefault to false (all other commands default it to true).\n\nAlso: how does this interact with notes.rewriteRef and the corresponding\nenv vars?  Why?\n\nHow does it interact with 'cherry-pick -n' if this is done in sequence,\neffectively squashing several commits (this use-case is actually\nsuggested by the manpage), if multiple source commits had notes?  Should\nit respect notes.rewriteMode (and by default concatenate)?  (I don't\nknow if the sequencer state is expressive enough already to carry this\nin a meaningful way across cherry-pick commands.)\n\nA commit message and some docs would be a nice idea, too.\n\n> diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\n> index f038f34..79c1219 100755\n> --- a/t/t3500-cherry.sh\n> +++ b/t/t3500-cherry.sh\n\nThis file starts out with\n\n  test_description='git cherry should detect patches integrated upstream\n\n  This test cherry-picks one local change of two into master branch, and\n  checks that git cherry only returns the second patch in the local branch\n  '\n\nSo either your tests should go to a different file or the description\nbecomes stale and needs to be updated.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"218714","messageId":"51a56aec503d7_807b33e18998da@nysa.mail","threadId":"33959","inReplyTo":"7vobbv119k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T02:41:48Z","receivedAt":"2013-05-29T02:41:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> > ---\n> >  builtin/revert.c  |   2 +\n> >  sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n> >  sequencer.h       |   2 +\n> >  t/t3500-cherry.sh |  32 +++++++++++++\n> >  4 files changed, 169 insertions(+), 3 deletions(-)\n> \n> \"git cherry-pick\" should help maintaining notes just like amend and\n> rebase, but how should this interact with notes.rewrite.<command>,\n> where the command is capable of doing this without an explicit\n> option once you tell which notes need to be maintained?\n> \n> Thomas Rast Cc'ed as he has been the primary force behind this line\n> of \"notes\" usability.\n> \n> It probably is not sensible to carry over note from a reverted\n> commit to its revert, but I didn't immediately spot how the patch\n> does this only for cherry-pick but not for revert (the codepath in\n> do_pick_commit() is shared between them, no?).\n\nThe option is enabled from the UI, and only 'git cherry-pick' has the\n--copy-notes option.\n\n-- \nFelipe Contreras\n"},{"id":"218715","messageId":"51a56bef1b9c2_807b33e1899991@nysa.mail","threadId":"33959","inReplyTo":"87mwrfx9ug.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T02:46:07Z","receivedAt":"2013-05-29T02:46:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Thomas Rast wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Thomas Rast Cc'ed as he has been the primary force behind this line\n> > of \"notes\" usability.\n> \n> Thanks for pointing this out to me.\n> \n> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n> >\n> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> >> ---\n> >>  builtin/revert.c  |   2 +\n> >>  sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n> >>  sequencer.h       |   2 +\n> >>  t/t3500-cherry.sh |  32 +++++++++++++\n> >>  4 files changed, 169 insertions(+), 3 deletions(-)\n> >\n> > \"git cherry-pick\" should help maintaining notes just like amend and\n> > rebase, but how should this interact with notes.rewrite.<command>,\n> > where the command is capable of doing this without an explicit\n> > option once you tell which notes need to be maintained?\n> \n> Since we already have the notes.rewrite.<command> convention, it would\n> seem the obvious choice to line it up with the others.  The main\n> bikeshedding opportunity is whether this should be an exception and\n> default to false (all other commands default it to true).\n> \n> Also: how does this interact with notes.rewriteRef and the corresponding\n> env vars?  Why?\n> \n> How does it interact with 'cherry-pick -n' if this is done in sequence,\n> effectively squashing several commits (this use-case is actually\n> suggested by the manpage), if multiple source commits had notes?  Should\n> it respect notes.rewriteMode (and by default concatenate)?  (I don't\n> know if the sequencer state is expressive enough already to carry this\n> in a meaningful way across cherry-pick commands.)\n\nFeel free to implement that. I'm just interested in 'git cherry-pick' being\nusable for 'git rebase' purposes.\n\n-- \nFelipe Contreras\n"},{"id":"218784","messageId":"877giixl4c.fsf@linux-k42r.v.cablecom.net","threadId":"33959","inReplyTo":"51a56bef1b9c2_807b33e1899991@nysa.mail","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-29T08:09:55Z","receivedAt":"2013-05-29T08:09:55Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Thomas Rast wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>> \n>> > Thomas Rast Cc'ed as he has been the primary force behind this line\n>> > of \"notes\" usability.\n>> \n>> Thanks for pointing this out to me.\n>> \n>> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n>> >\n>> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> >> ---\n>> >>  builtin/revert.c  |   2 +\n>> >>  sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n>> >>  sequencer.h       |   2 +\n>> >>  t/t3500-cherry.sh |  32 +++++++++++++\n>> >>  4 files changed, 169 insertions(+), 3 deletions(-)\n>> >\n>> > \"git cherry-pick\" should help maintaining notes just like amend and\n>> > rebase, but how should this interact with notes.rewrite.<command>,\n>> > where the command is capable of doing this without an explicit\n>> > option once you tell which notes need to be maintained?\n>> \n>> Since we already have the notes.rewrite.<command> convention, it would\n>> seem the obvious choice to line it up with the others.  The main\n>> bikeshedding opportunity is whether this should be an exception and\n>> default to false (all other commands default it to true).\n>> \n>> Also: how does this interact with notes.rewriteRef and the corresponding\n>> env vars?  Why?\n>> \n>> How does it interact with 'cherry-pick -n' if this is done in sequence,\n>> effectively squashing several commits (this use-case is actually\n>> suggested by the manpage), if multiple source commits had notes?  Should\n>> it respect notes.rewriteMode (and by default concatenate)?  (I don't\n>> know if the sequencer state is expressive enough already to carry this\n>> in a meaningful way across cherry-pick commands.)\n>\n> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n> usable for 'git rebase' purposes.\n\nWhich would have been obvious to all but the most casual readers, eh?\n\nWe've been over this already:\n\n  The body should provide a meaningful commit message, which:\n\n    . explains the problem the change tries to solve, iow, what is wrong\n      with the current code without the change.\n\n    . justifies the way the change solves the problem, iow, why the\n      result with the change is better.\n\n    . alternate solutions considered but discarded, if any.\n\nI'll gladly review your patches again once you have done that.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"218785","messageId":"CAMP44s3vkJCr2f2zsZU++j4Wqxuefmpt8BQ9dJoP=JtTB=rgkQ@mail.gmail.com","threadId":"33959","inReplyTo":"877giixl4c.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T08:19:12Z","receivedAt":"2013-05-29T08:19:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n>> usable for 'git rebase' purposes.\n>\n> Which would have been obvious to all but the most casual readers, eh?\n\nMy motivations are irrelevant, the patch is good as it is.\n\n> We've been over this already:\n>\n>   The body should provide a meaningful commit message, which:\n>\n>     . explains the problem the change tries to solve, iow, what is wrong\n>       with the current code without the change.\n\nObvious; there is no support to copy notes.\n\n>     . justifies the way the change solves the problem, iow, why the\n>       result with the change is better.\n\nObvious: because it now actually copies the notes.\n\n>     . alternate solutions considered but discarded, if any.\n\nThere are no alternate solutions.\n\n-- \nFelipe Contreras\n"},{"id":"218788","messageId":"87y5ayqivi.fsf@linux-k42r.v.cablecom.net","threadId":"33959","inReplyTo":"CAMP44s3vkJCr2f2zsZU++j4Wqxuefmpt8BQ9dJoP=JtTB=rgkQ@mail.gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-29T08:40:17Z","receivedAt":"2013-05-29T08:40:17Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n>>> usable for 'git rebase' purposes.\n>>\n>> Which would have been obvious to all but the most casual readers, eh?\n>\n> My motivations are irrelevant, the patch is good as it is.\n\nYou fooled both Junio (AFAICT anyway) and me, who both reviewed the\npatch under the assumption that it implements note copying *along the\nlines of existing note copying*.  This proved to be a wrong, and\ntime-wasting, assumption.\n\nI think all the points in this discussion have been made in the lengthy\nthread about remote-{bzr,hg} already.  So much so in fact that your\ncontinuing defiance of the project standards, knowing that they will\nelicit exactly this response, amounts to trolling.\n\nSo until this changes, my $0.02 is a blanket NAK and a refusal to spend\nmy time reviewing.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"218799","messageId":"CAMP44s2aAvzeTtHb=rVP1TrCMcQjyqN6O7n1DYZLx0uJrm1j+w@mail.gmail.com","threadId":"33959","inReplyTo":"87y5ayqivi.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T11:18:33Z","receivedAt":"2013-05-29T11:18:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 29, 2013 at 3:40 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>>> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n>>>> usable for 'git rebase' purposes.\n>>>\n>>> Which would have been obvious to all but the most casual readers, eh?\n>>\n>> My motivations are irrelevant, the patch is good as it is.\n>\n> You fooled both Junio (AFAICT anyway) and me, who both reviewed the\n> patch under the assumption that it implements note copying *along the\n> lines of existing note copying*.  This proved to be a wrong, and\n> time-wasting, assumption.\n\nWhatever arbitrary rules you are talking about, they are not codified in tests.\n\nIf you care so much about \"*the lines of existing note copying*\", why\ndon't you implement tests that check for those? This not only would\nprevent that some shmuck who is not well versed in the tradition of\n\"the lines of existing note copying\" from breaking that tradition,\nwhich is precisely what tests are for.\n\nIf this was done, we wouldn't be \"time-wasting\" here.\n\nThis patch makes 'git cherry-pick' pass all the tests that 'git\nrebase' passes. Period.\n\nEven if it was against \"the lines of existing note copying\", it's much\nbetter than the current situation, which is to do ABSOLUTELY NOTHING.\n\nIf you were a productive person, you would take this patch and\nimplement the code that makes it align to \"the lines of existing note\ncopying\" that you care so much about, which should be easy, if the\nnote framework was properly implement, but you won't.\n\nWhy isn't this implemented?\n\nvoid copy_notes_for_rewrite(const char *rewrite_cmd, struct rewritten *list);\n\nIf there was such a thing, other clients wouldn't need to implement\ntheir own methods of copying notes.\n\nBut you didn't implement that, did you?\n\nYou are punishing me because the notes framework is lacking, and\nbecause the testing framework is missing what you think is the proper\nbehavior.\n\nStrike that, you are punishing the users.\n\n-- \nFelipe Contreras\n"},{"id":"218803","messageId":"874ndmqate.fsf@linux-k42r.v.cablecom.net","threadId":"33959","inReplyTo":"CAMP44s2aAvzeTtHb=rVP1TrCMcQjyqN6O7n1DYZLx0uJrm1j+w@mail.gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-05-29T11:34:21Z","receivedAt":"2013-05-29T11:34:21Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Wed, May 29, 2013 at 3:40 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>>> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n>>>>> usable for 'git rebase' purposes.\n>>>>\n>>>> Which would have been obvious to all but the most casual readers, eh?\n>>>\n>>> My motivations are irrelevant, the patch is good as it is.\n>>\n>> You fooled both Junio (AFAICT anyway) and me, who both reviewed the\n>> patch under the assumption that it implements note copying *along the\n>> lines of existing note copying*.  This proved to be a wrong, and\n>> time-wasting, assumption.\n>\n> Whatever arbitrary rules you are talking about, they are not codified in tests.\n\nTests or code don't have a thing to do with it.  This is about how you\nare presenting your changes to the rest of the git community.  As\nevidenced above, said presentation is not clear enough to communicate\nyour goals to at least two experienced git developers (if I may say so\nmyself).  How are we supposed to review a change if it is not even clear\nwhat goal it satisfies?\n\nAgain: I'll be happy to review your proposed changes if and when you\nresend the series with commit messages.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"218805","messageId":"CAMP44s2de-kNnxQvLNE5QrDFYMX=Omyu61+J=Dbxp0of3iYGxg@mail.gmail.com","threadId":"33959","inReplyTo":"874ndmqate.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T11:56:54Z","receivedAt":"2013-05-29T11:56:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 29, 2013 at 6:34 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Wed, May 29, 2013 at 3:40 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>>\n>>>>>> Feel free to implement that. I'm just interested in 'git cherry-pick' being\n>>>>>> usable for 'git rebase' purposes.\n>>>>>\n>>>>> Which would have been obvious to all but the most casual readers, eh?\n>>>>\n>>>> My motivations are irrelevant, the patch is good as it is.\n>>>\n>>> You fooled both Junio (AFAICT anyway) and me, who both reviewed the\n>>> patch under the assumption that it implements note copying *along the\n>>> lines of existing note copying*.  This proved to be a wrong, and\n>>> time-wasting, assumption.\n>>\n>> Whatever arbitrary rules you are talking about, they are not codified in tests.\n>\n> Tests or code don't have a thing to do with it.  This is about how you\n> are presenting your changes to the rest of the git community.  As\n> evidenced above, said presentation is not clear enough to communicate\n> your goals to at least two experienced git developers (if I may say so\n> myself).  How are we supposed to review a change if it is not even clear\n> what goal it satisfies?\n\nMy goals are irrelevant. This patch is good.\n\nIt implements a missing feature, if you don't like how it is implemented it:\n\na) Implement the code in the note framework that does it properly so\nother people can just call that.\n\nb) Implement the tests for other commands that check that the behavior\nyou talk about does happen indeed.\n\nc) Implement it yourself\n\nThis has nothing to do with the way I presented the patch. I presented\nthe patch as I thought it was; implementing the note copying as it was\nintended. Now you came along and say I wasted your time because I\ndidn't say I implemented it wrongly, and you assume bad faith and what\nnot.\n\nThis wouldn't have happened because you didn't do a) or b). I realized\nthat by replacing 'am' with 'cherry-pick' in 'git rebase'  the notes\nwere not copied, according to the testing framework, so I implemented\nthat, and the tests pass. Now you come along and say it should\nimplemented some note.rewrite.command stuff, but the tests didn't\ncheck for that, so how was I supposed to know?\n\nAnd then you have the audacity to claim that *I*, the one who just\nwrote a bunch of code to implement a missing feature, is wasting\n*your* time, you, the one who is simply replying to an email and\nshooting from the hip.\n\n> Again: I'll be happy to review your proposed changes if and when you\n> resend the series with commit messages.\n\nI won't, I'll keep working on my actual objective.\n\nPlus, this patch does have a commit message, and the commit message\nsays *EXACTLY* what the patch is doing: add support to copy notes.\n\nIf *you* are interested in certain behavior, why don't you lift a\nfinger and do something to make sure that such behavior is easy to\nimplement and the test framework actually checks for that?\n\n-- \nFelipe Contreras\n"},{"id":"218809","messageId":"CALkWK0k5+2nebRLZfWFpxyPmptV_Qo3sKwgpbpvN-bK6_S6W3w@mail.gmail.com","threadId":"33959","inReplyTo":"87y5ayqivi.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T12:09:58Z","receivedAt":"2013-05-29T12:09:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> So until this changes, my $0.02 is a blanket NAK and a refusal to spend\n> my time reviewing.\n\nThen don't review the damn thing.  With Felipe, I have the following\nrule of thumb: make some concrete suggestions and forget about\nfollow-ups.  He's not going to accept any general guidelines, unless\nyou're quoting Documentation/SubmittingPatches (and even then, it's\nsubject to interpretation); so provide a commit message and hope that\neither he or Junio will use it.  There is no guarantee that he will\ntake any of your suggestions, no matter how sensible you think they\nmight be.  However, he is a productive programmer, and submits fixes\nto real issues.  He's stubborn, and we can't do much to change that:\njust learn to work with him.  I'm disappointed that I have to point\nthis out: haven't you learnt anything from previous discussions with\nhim?\n\nFelipe, I suggest you put this in your commit message:\n\n   This patch implements --copy-notes for 'git cherry-pick' so it can\ncopy notes in the same way that 'git rebase' does.\n\nThat is, if it's not too much trouble.\n\nStop this back-and-fourth nonsense, both of you.  It's degrading the\ncommunity, and hitting everyone's inboxes with garbage.\n"},{"id":"218821","messageId":"CAMP44s2tvUs813hrheWpWXR2G4kMJL4rscEtynaKYvw5a6HCHA@mail.gmail.com","threadId":"33959","inReplyTo":"CALkWK0k5+2nebRLZfWFpxyPmptV_Qo3sKwgpbpvN-bK6_S6W3w@mail.gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T13:18:22Z","receivedAt":"2013-05-29T13:18:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 29, 2013 at 7:09 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Thomas Rast wrote:\n>> So until this changes, my $0.02 is a blanket NAK and a refusal to spend\n>> my time reviewing.\n>\n> Then don't review the damn thing.  With Felipe, I have the following\n> rule of thumb: make some concrete suggestions and forget about\n> follow-ups.\n\nHe didn't make any suggestions.\n\n> He's not going to accept any general guidelines, unless\n> you're quoting Documentation/SubmittingPatches (and even then, it's\n> subject to interpretation); so provide a commit message and hope that\n> either he or Junio will use it.  There is no guarantee that he will\n> take any of your suggestions, no matter how sensible you think they\n> might be.\n\nThis is bullshit.\n\nLet's look at some of the suggestions you have made:\n\n== git-related ==\n\n> s/l/line/?\n\nI said fine, and I implemented it.\n\n> Still no -CCC?\n\nI said I forgot, and I implemented it.\n\nWhat you are really complaining about is that I don't agree with\n*every* single suggestion you make. And since you made them, they must\nbe sensible, and single I don't agree with you, I must not be\nsensible, is that right?\n\nAnd stop bringing irrelevant garbage to this discussion. The\ndiscussion about the coding-style not about guidelines, because there\nis no guideline for that open parenthesis, ad obviated by the fact\nthat there's over *FIVE HUNDRED* instances where it's not aligned that\nway.\n\nNobody is denying the notes.rewritten.* guideline here, I didn't\nbecause that is *actually* a guideline. So your comment about\nguidelines is an irrelevant straw man.\n\n> However, he is a productive programmer, and submits fixes\n> to real issues.  He's stubborn, and we can't do much to change that:\n> just learn to work with him.  I'm disappointed that I have to point\n> this out: haven't you learnt anything from previous discussions with\n> him?\n>\n> Felipe, I suggest you put this in your commit message:\n>\n>    This patch implements --copy-notes for 'git cherry-pick' so it can\n> copy notes in the same way that 'git rebase' does.\n>\n> That is, if it's not too much trouble.\n>\n> Stop this back-and-fourth nonsense, both of you.  It's degrading the\n> community, and hitting everyone's inboxes with garbage.\n\nThanks.\n\nBut let's take a step backwards, what are we trying to achieve here?\nWe are trying to improve Git, and the indisputable fact is that 'git\ncherry-pick' is missing a way to copy the notes.\n\nIt's indisputable that this patch implements that, and I did it by\nfollowing existing code, and by running the whole test suit for 'git\nrebase'. I've done my job already.\n\nThomas Rast doesn't like the way this is implemented, and nothing in\nthe commit message would change that.\n\nThis is was a sensible community, you would stop ganging up on me,\nThomas Rast would implement copy_notes_for_rewrite(), and add tests\nfor other commits (git am, git rebase) to check that the functionality\nhe claims to be so worried about is working properly.\n\nAnd this was a sensible community you wouldn't complain about me\nchoosing how to spend my time however I see fit.\n\nI did some work, I sent a patch, Thomas Rast has some issues, I'm not\ninterested enough in this patch to investigate them, I work on\nsomething else. What's wrong with that?\n\nEventually I might come back to this patch, and eventually I might\nimplement copy_notes_for_rewrite, and I might implement the tests that\ncheck for the behavior that I missed, if nobody beats me to it, which\nis usually the case, but I think Thomas should put his personal issues\naside, put his money where his mouth is, and implement it himself.\n\nThere's nothing wrong with me choosing how best to spend my time. Really.\n\n-- \nFelipe Contreras\n"},{"id":"218837","messageId":"CALkWK0kK+Mbscsue0wwU5QDKXCrw5Q078MCdOVCoJnMZq7T6UA@mail.gmail.com","threadId":"33959","inReplyTo":"CAMP44s2tvUs813hrheWpWXR2G4kMJL4rscEtynaKYvw5a6HCHA@mail.gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T13:48:48Z","receivedAt":"2013-05-29T13:48:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> What you are really complaining about is that I don't agree with\n> *every* single suggestion you make. And since you made them, they must\n> be sensible, and single I don't agree with you, I must not be\n> sensible, is that right?\n\nOh, I have no problems: I reviewed git-related, and it resulted in\nmassive simplifications and improvements; those threads didn't run\nwild.  I was mainly criticizing Thomas' review style (when it comes to\nreviewing your patches in particular), and was prodding him to \"make\nconcrete suggestions\" in a one-email review, and leave it at that.\nWhat started out as a pointer to the guidelines:\n\nThomas Rast wrote:\n> We've been over this already:\n>\n>   The body should provide a meaningful commit message, which:\n\nhas resulted in this thread running wild.\n\n> There's nothing wrong with me choosing how best to spend my time. Really.\n\nOfcourse you are.  You have arguably spent it very productively\nsolving a lot of user issues (especially remote-bzr).\n\nPersonally, I try to do the minimum amount of boring work required to\nmake sure that a good series gets in.  Sometimes this is a little high\n(for a recent example, see my pickaxe-doc series).  The result being\nthat I just won't work on documentation in the future because doing\niterations is so piss boring: the git community needs to recognize\nthis problem and make amends.\n"},{"id":"218842","messageId":"CAMP44s3TKCtn8f_zU1geFcmgCFVkKF+ipBFkV+cSa+6CtLX0gA@mail.gmail.com","threadId":"33959","inReplyTo":"CALkWK0kK+Mbscsue0wwU5QDKXCrw5Q078MCdOVCoJnMZq7T6UA@mail.gmail.com","subject":"Re: [PATCH 1/3] cherry-pick: add support to copy notes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-29T14:01:11Z","receivedAt":"2013-05-29T14:01:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 29, 2013 at 8:48 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n\n>> There's nothing wrong with me choosing how best to spend my time. Really.\n>\n> Ofcourse you are.  You have arguably spent it very productively\n> solving a lot of user issues (especially remote-bzr).\n>\n> Personally, I try to do the minimum amount of boring work required to\n> make sure that a good series gets in.  Sometimes this is a little high\n> (for a recent example, see my pickaxe-doc series).  The result being\n> that I just won't work on documentation in the future because doing\n> iterations is so piss boring: the git community needs to recognize\n> this problem and make amends.\n\nI don't mind doing as many iterations as it takes, as long as it's\nabout meaningful issues.\n\nI don't particularly enjoy, but I'm OK with discussing non-meaningful issues.\n\nWhat I'm not OK with is disagreements that end up breaking the\ncommunication, specially when a crystal-clear case has been made\n(IMO), and the patch goes to limbo for no reason. That I think is a\nreal problem.\n\nFor this particular patch, I don't care if it goes in at the moment. I\nhave something big on the pipeline, and I would rather drop this than\nloose penguin points, although I probably don't have any left.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}