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

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

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 :)

review: Approve

« Back to merge proposal