Code review comment for lp://staging/~spiv/bzr-builder/refactor

On Wed Nov 18 03:45:21 UTC 2009 Andrew Bennetts wrote:
> Andrew Bennetts has proposed merging lp:~spiv/bzr-builder/refactor into
> lp:bzr-builder with lp:~lifeless/bzr-builder/bug-469874 as a prerequisite.
>
> Requested reviews:
> James Westby (james-w)
>
>
> This basically a replaces a list of 2-tuples where (*, None), (None, *) and
> (None, None) all mean different things, and replaces them with objects. This
> also replaces if/elif/... blocks about those cases with good polymorphism.

Thanks, code elegance is always appreciated.

> The names I gave to the objects and methods can probably be improved a fair
> bit, and I'm happy to do that if you have any suggestions about what these
> items should be called.

I think the names are good.

My only concern is with .as_tuple(), as the tuple isn't something that has
a natural definition based on the object, and it may make no sense for
some instructions. However, as these were issues with the original code
and doing this means less test changes are needed then I'm happy to
merge it, thanks.

Thanks,

James