launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

> This looks alright, but I'm not sure I see the value in the completeness
> expression being a class method on the Specification class--it seems like
> something that could just be defined as a clause separately in that code

Which code?  Sprint.specification_filter?  It should not be defined there, because it is more generally useful.  We will need it when we convert other implementations of IHasSpecifications.specifications to Storm.

> rather than a class method, and then used as needed. Is there some other
> reason for this implementation?

I thought it was valuable to have it directly underneath the non-Storm version, Specification.completeness.

> Also: I concur with the lack of a need for an explicit implementation for
> Priority, but that should probably be added in a comment in the relevant block
> of code.

There is a comment:
212	+ # fall back to default, which is priority, descending.

Does it need improvement?
-- 
https://code.launchpad.net/~abentley/launchpad/storm-sprint-queries/+merge/126770
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.