Code review comment for lp:~albaguirre/mir/add-menu-api

Tests have some
+ ASSERT_TRUE(mir_surface_is_valid(menu));
this would be better with mir_test/validity_matchers.h and:
+ ASSERT_THAT(menu, IsValid())
(this gets you a helpful message when the assertion fails

What are the semantics of mir_edge_attachment_none? I don't think that value makes sense?

I agree with Robert; having an adjacency rect set but not a parent or edge_attachment should throw from SessionMediator. As long as our client library isn't buggy it will never send such a message, but it's cheap and useful to check.

Hm. That actually suggests a MessageValidator type interface, dispatching based on surface_type. Probably not for this MP, though.

I also like Robert's rewording of the documentation comment (although I'd replace ‘may be specified’ with ‘is specified’).

I'm only blocking on “what does mir_edge_attachment_none mean?” Everything else is nice-to-have.