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

Hi,

Ramkumar Ramachandra wrote:

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