git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
Date: Wed, 11 Jan 2012 07:40:12 -0600	[thread overview]
Message-ID: <20120111134011.GH32173@burratino> (raw)
In-Reply-To: <CALkWK0mv2jzmDA==pJg5R4jH0yxo=OopYM_WzAWusiffnb+4HQ@mail.gmail.com>

Ramkumar Ramachandra wrote:

> We'd be prematurely locking ourselves into a design where we can't
> tell which top-level command issued the continue/ abort

The .git/sequencer/todo file already doesn't record which top-level
command initiated the sequence and doesn't seem to operate under a
model in which that is a useful question.  Honestly, that's my only
objection to the "git revert --continue during git cherry-pick" check.

I think it is not premature to think about whether that matters.  I've
already said a little about related cases where it seemed to matter
but there was instead something else at play.  Can you offer some
examples of how people might use the "git cherry-pick" / "git revert"
commands and get stuck or run into trouble, and how git can help them?

[...]
>                                                           My sincere
> suggestion is to procrastinate the problem until we have a tighter
> usecase (a new top-level command or action added, for instance).

Thinking carefully about sequencer use cases also seems like a good
idea.  Is it intended that "git am" and "git rebase" should be
reimplemented on top of the sequencer?  Do you have goals in mind for
commands like "git sequence --step" that could be used to examine,
influence, and carry out git's idea of what should happen next?

If we have no use case, then there's no reason to change the code.  It
already works[*] for cherry-pick and revert.  I think we shouldn't be
moving this code to sequencer.c or cleaning up the API to suit other
commands (e.g., introducing two ways to sane "am I picking or
reverting") without having one in mind.

[*] Modulo bugs and some missing features such as --skip.

  reply	other threads:[~2012-01-11 13:35 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-08 12:27 [PATCH 0/6] The move to sequencer.c Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 1/6] revert: move replay_action, replay_subcommand to header Ramkumar Ramachandra
2012-01-08 19:31   ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 2/6] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-08 19:34   ` Jonathan Nieder
2012-01-08 19:53     ` Ramkumar Ramachandra
2012-01-08 20:09       ` Jonathan Nieder
2012-01-08 20:07         ` Ramkumar Ramachandra
2012-01-08 20:48           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 3/6] revert: don't let revert continue a cherry-pick Ramkumar Ramachandra
2012-01-08 19:37   ` Jonathan Nieder
2012-01-08 20:03     ` Ramkumar Ramachandra
2012-01-08 20:22       ` Jonathan Nieder
2012-01-08 20:28         ` Ramkumar Ramachandra
2012-01-08 20:45           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 4/6] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-08 19:40   ` Jonathan Nieder
2012-01-08 20:17     ` Ramkumar Ramachandra
2012-01-08 21:40       ` Jonathan Nieder
2012-01-08 21:55         ` Jonathan Nieder
2012-01-10  3:40           ` Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 5/6] revert: report fine-grained error messages from insn parser Ramkumar Ramachandra
2012-01-08 20:07   ` Jonathan Nieder
2012-01-08 20:16     ` Ramkumar Ramachandra
2012-01-08 21:33       ` Jonathan Nieder
2012-01-10 15:24         ` Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 6/6] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-08 20:38   ` Jonathan Nieder
2012-01-10 15:21     ` Ramkumar Ramachandra
2012-01-08 19:28 ` [PATCH 0/6] The move to sequencer.c Jonathan Nieder
2012-01-08 19:51   ` Ramkumar Ramachandra
2012-01-08 20:43     ` Jonathan Nieder
2012-01-10 16:13 ` [PATCH v2 0/8] " Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 1/8] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-10 18:27     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 2/8] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-10 18:38     ` Jonathan Nieder
2012-01-11  4:02       ` Ramkumar Ramachandra
2012-01-11  4:17         ` Ramkumar Ramachandra
2012-01-11  5:04           ` Jonathan Nieder
2012-01-11  5:14             ` Ramkumar Ramachandra
2012-01-11  5:26               ` Ramkumar Ramachandra
2012-01-11  5:49                 ` Jonathan Nieder
2012-01-11  9:19                   ` Ramkumar Ramachandra
2012-01-11  9:52                     ` Jonathan Nieder
2012-01-11 10:11                       ` Ramkumar Ramachandra
2012-01-11 13:40                         ` Jonathan Nieder [this message]
2012-01-11 13:18               ` Jonathan Nieder
2012-01-11 16:39                 ` Ramkumar Ramachandra
2012-01-11 16:47                   ` Jonathan Nieder
2012-01-11 16:52                     ` Ramkumar Ramachandra
2012-01-11 18:15                     ` [PATCH v3 0/2] The move to sequencer.c Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 1/2] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 2/2] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-11 18:40                       ` [PATCH v3 0/2] The move to sequencer.c Jonathan Nieder
2012-01-10 16:13   ` [PATCH 3/8] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 4/8] revert: separate out parse errors logically Ramkumar Ramachandra
2012-01-10 19:03     ` Jonathan Nieder
2012-01-11 12:38       ` Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 5/8] revert: report fine-grained errors from insn parser Ramkumar Ramachandra
2012-01-11 12:44     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 6/8] sha1_name: introduce getn_sha1() to take length Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 7/8] revert: use getn_sha1() to simplify insn parsing Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 8/8] sequencer: factor code out of revert builtin Ramkumar Ramachandra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120111134011.GH32173@burratino \
    --to=jrnieder@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).