[PATCH 1/4] Re: Do away with UI plugins and simplify calling the GUI

[Nicolas Sebrecht]

On Thu, Dec 23, 2010 at 01:06:16PM -0500, Leif Walsh wrote:
> On Wed, Dec 22, 2010 at 9:27 AM, Sebastian Spaeth <Sebastian at sspaeth.de> wrote:
> 
> > Blinkenlights was not optimal for various reasons, e.g. Exceptions
> > would be hidden as the screen immediately reset itself after exit. It
> > also frequently messed up the screen when offlineimap crashed.
> 
> Please don't completely remove a feature because you don't like it or
> think it has a couple bugs.  The typical way to handle these things is
> to either fix it or file a bug report so someone else can, and if
> nobody does for a few months or a year, you can mark it deprecated and
> remove it after another year or so.

This has already been discussed. We came to the conclusion that most UI
aren't used at all and don't help reducing the maintenance code. This is
why we decided to simply remove some UI. I think it's better to NOT have
broken UI than keeping them along.

Because of the feature changes, I don't plan to merge such a patch
before the next major release (we could go from v6.3.x to 6.4.0).

But you don't say what looks more critical to me: do you really use the
Blinkenlights UI? In which use case another UI can't do the job?

> > Simplify the names of the GUIs to something that a mortal can actually
> > remember, make them "TTY, Basic, Quiet, Machine" with TTY being the
> > default.
> 
> You are introducing changes that will break existing configuration.
> Have you documented this in enough places?

The documentation is currently frozen for the time I finish the SGML to
RST migration. This is a work in progress. I hope to submit and then
merge it before the next stable release.

> Have you documented this in enough places?  Does the code fail
> gracefully given an old config?  Echoing Nicolas, this patch needs to
> be broken up into smaller pieces, in particular so that I can answer
> these questions for myself by reading the code.  With a patch this big
> affecting this many pieces of code and features, I don't want to look
> through it to figure these out.

I didn't read the patch sent for the same reason (not because the goal
is not interesting). Will see in the next round.

> It also looks like you have a bunch of "code style" changes sprinkled
> in.  It is best (though I understand, annoying) to separate these from
> "application logic" changes, for the same reason.  I would rather not
> read every line of your patch to determine what it does, it would be
> nicer if every line in a patch could be a change to make the same one
> thing happen, because that's the topic.

Some code style changes are good to include to other patches while
touching an area of the code (typically when the style change affects
the same code you're touching). I don't like pure code style patches
that much because of the risk to include new failures. Patches
introcucing failures (perhaps even tested) because of code style only
changesets are good to avoid.  I can't say because I didn't read the
patch of Sebastian.

That said, I won't be here as strict as I am (or as you will hit) in
other projects: the active developers community around OfflineIMAP is
not so wide.

But you should tell exactly what you have in mind by quoting the
changesets. I ask authors to send patches by email for good reasons, and
one of the main reason is to help reviewers to comment changes EASILY.

Sebastian do send his patches by mail. It would be very appreciated to
not ignore this workload he did _for us_.

-- 
Nicolas Sebrecht