Code review comment for lp:~brian-murray/launchpad/595124

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

I don't mind the backward incompatibility either. I think Brian is one of the heavy user of that API and if that's fine by him, I wouldn't second guess him here.

I have another concern which isn't introduce by this branch, which is that this means that sending a bugs representation makes an additional query to get the value of the can_expire attribute. I think this is very bad from a performance point of view. Especially when navigating through bug searches.

A better API would be to turn this into a collection exposed on the context and return the expirable bug tasks.

And just remove can_expire from the model. Maybe that's what we should do now? Mark it as removed, export only isExpirable() which can be used to retrieve the information if really needed. Exporting the set of expirable bugs might be done at another time (not sure if the IBugTarget has findExpirableBugs in it already).

« Back to merge proposal