Mir

Code review comment for lp:~andreas-pokorny/mir/pixel-format-utils

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> (4) It's dangerous to assume mir_pixel_format_invalid has a lower integer
> value than the valid pixel formats:
> mir_pixel_format_invalid < format
> unless the order is somehow guaranteed by the name. The same is possibly also
> true about "mir_pixel_formats" if and when we choose to encode more
> information into the mir_pixel_format_ values.
>
> You can make such assumptions more safely if it's defined as a macro adjacent
> to the definition of MirPixelFormat. Then you can see both definitions side-
> by-side and the risk of one regressing independently of the other is much
> lower.

As soon as somebody changes the enumeration - like reordering - the tests will probably fail and a better implementation is necessary. Actually the current implementation is just the result of doing tdd up to the point where those tests run. I just assumed that nobody will make changes that are not driven by tests.

I would like to redesign the enumeration and add support for more formats, but as you already said in a previous review of the same code that can be added later.

Maybe it is better to make bigger review..

« Back to merge proposal