Jakub Narębski <jna...@gmail.com> writes:

>> diff --git a/sequencer.c b/sequencer.c
>> index 06759d4..3398774 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -709,6 +709,8 @@ static int read_and_refresh_cache(struct replay_opts 
>> *opts)
>>  struct todo_item {
>>      enum todo_command command;
>>      struct commit *commit;
>> +    const char *arg;
>> +    int arg_len;
> Why 'arg', and not 'oneline', or 'subject'?
> I'm not saying it is bad name.

I am not sure what the "commit" field of type "struct commit *" is
for.  It is not needed until it is the commit's turn to be picked or
reverted; if we end up stopping in the middle, parsing the commit
object for later steps will end up being wasted effort.

Also, when the sequencer becomes one sequencer to rule them all, the
command set may contain something that does not even mention any
commit at all (think: exec).

So I am not sure if we want a parsed commit there (I would not
object if we kept the texual object name read from the file,
though).  The "one sequencer to rule them all" may even have to say
"now give name ':1' to the result of the previous operation" in one
step and in another later step have an instruction "merge ':1'".
When that happens, you cannot even pre-populate the commit object
when the sequencer reads the file, as the commit has not yet been
created at that point.

Reply via email to