Code review comment for lp:~thumper/launchpad/blueprint-dependency-vocabulary

Revision history for this message
Tim Penhey (thumper) wrote :

On Tue, 15 Mar 2011 12:13:40 you wrote:
> Review: Approve
> Hi Tim,
>
> This basically looks fine. The only comments I have are:
>
> * I think the ISpecification check is a bit over the top (already said
> this on IRC), * I think _order_by.__doc__ could do with expansion (or
> deletion -- the current docstring doesn't really add anything) * I think
> "_exclude_blocked_query" would be a better name than "_blocked_subselect"
> (the latter sounds a bit like it returns the blocked specs) * "Not" is
> imported but not used in
> ./lib/lp/blueprints/vocabularies/specificationdependency.py
>
> "make lint" also reports a few long lines, up to you if you care :)

Addressed, and thanks.

« Back to merge proposal