Code review comment for lp:~sinzui/charmworld/support-gui-charm-urls

Revision history for this message
Benji York (benji) wrote :

The branch looks good. The tests especially nice.

On lines 233, 255, and 262 you could use self.assertIn and get slightly
better diagnostics if the assertion ever fails.

Unpacking the comments in the routing tests would be nice. I can pretty
much understand them in the context of the branch, but I suspect someone
coming in cold would have to ruminate a bit to see what they are saying.
For example, test_route_personal_charm_with_revision's comment of

    # /~owner/series/charm-1 route find the charm.

Might be expanded to

    # A non-promulgated charm's route of the form /~owner/series/charm-1
    # specifies the charm with a charm store ID of
    # cs:~owner/series/charm.

review: Approve

« Back to merge proposal