Code review comment for lp:~alan-griffiths/mir/add-mir_surface_spec_attach

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> OK on the first issue. Although "helps with an immediate problem without
> getting in the way of further enhancements" is a benefit that's also
> contradictory to good engineering: Everything you write should ideally have
> multiple uses so that functionality grows exponentially faster than the code
> size. We're failing on that part.

Having a function that (re)specifies the attachment without reference to the surface type does provide multiple uses. If we later (as one suggestion has it) enhance MirEdgeAttachment it automatically gains that functionality.

> So Abstain except for Cemil's comment...
> I agree that mir_surface_spec_attach is not an adequate name on first glance.
> You should be able to roughly figure out the purpose from the function name.

I'm in the market for a better name, but I don't like the current suggestions as much as the proposed.

« Back to merge proposal