Thanks for the review Henninge.
All your suggestions are useful and I'll act on them before landing this branch. The one about chaining however I won't - we end up with 3 lines with poor indenting (its too long).
The implements thing is interesting; it turns out that the security rules are not cross checking the implements() - the interfaces primary job is then to abstract out the method signature. I'll add them though we don't strictly need them - and I bet we're missing other implements within the code base.
The admin team checking is doing visibility checks - same as private bugs. The order_by thing nukes the default order_by - storm sets a query to find_spec.default_order, and we really want no order by to ensure we get the best query plan. (yes, its weird that the default is possibly ordered) - I'm just using belts and bracers to avoid surprises.
Thanks for the review Henninge.
All your suggestions are useful and I'll act on them before landing this branch. The one about chaining however I won't - we end up with 3 lines with poor indenting (its too long).
The implements thing is interesting; it turns out that the security rules are not cross checking the implements() - the interfaces primary job is then to abstract out the method signature. I'll add them though we don't strictly need them - and I bet we're missing other implements within the code base.
The admin team checking is doing visibility checks - same as private bugs. The order_by thing nukes the default order_by - storm sets a query to find_spec. default_ order, and we really want no order by to ensure we get the best query plan. (yes, its weird that the default is possibly ordered) - I'm just using belts and bracers to avoid surprises.