Re: [PATCH 00/13] Sequencer with continuation features

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Ramkumar Ramachandra wrote:

> I think I can safely say that
> I'm quite happy with the general state of this series now.

It's certainly starting to shape up.  I'll quickly review the style
and commit messages now, since the part I was lobbying for most has
been dealt with (hoorah!) and the rest of the substantive part seems
to have some potential changes queued up if I understand correctly.
In particular, I have high hopes for changes rippling through once
tests get added to make the detailed behavior more clear. :)

General rule of thumb about style, especially commit messages: if
something looks wrong, it is.  Which is to say, the goal is to make
the code easy to read and hack on, and part of that means avoiding
that moment of surprise that might interfere with someone getting on
with the task at hand.  If I seem to be not making sense, please as
usual do not assume I am right, but think carefully about it and
(ideally) gently correct me if I'm wrong.

Okay, on to the patches.  The general shape of the series is a little
odd --- some die() elimination, removal of globals in preparation for
making this a reusable API, and then the title feature that does not
rely on any of the patches before it.  Still, it makes a kind of sense
from the point of view of development:

 1. First, getting to know the API and using that process to come
    up with obvious improvements.

 2. Next, implementing the new feature.

If I had been writing it, I would have rebased the last 4 patches
against master as a new series (so they could be integrated more
quickly), but I don't mind reading it this way.

Regards,
Jonathan