Code review comment for lp:~gary-wzl77/scope-aggregator/force_display_results

Revision history for this message
Gary.Wang (gary-wzl77) wrote :

Yes,
It's a workaround for a special case(Bollywood aggregator).
It doesn't support results search for child scopes in an old version,
except 3rd-party scope(cinema).
I've updated all other child scope to support query when the request comes
from aggregator.
It looks good so far.
Let's keep lean generic library and not add workarounds to make this
library fat.
I think that makes it easier for maintains as well.
Thanks.

On Fri, Aug 21, 2015 at 1:27 AM, Kyle Nitzsche <email address hidden>
wrote:

> Review: Needs Fixing
>
> Hi Gary,
>
> Is this functionality required for the BW scope (to work with a specific
> child scope we did not write)?
>
> If this functionality is required then we should merge it because we do
> not want to fork scope-aggegator, and BW may need to be updated later from
> s-a new releases.
>
> However, I think this whole "only_in_search" functionality is a work
> around (hack) that we needed for BW to work with a child scope we did not
> write (right?). If so, I doubt we will ever need this functionality again.
> Therefore: if you agree with this analysis, I propose that you modify the
> README and add a comment saying this json key is "deprecated and exists
> only to support a special case".
>
> --
>
> https://code.launchpad.net/~gary-wzl77/scope-aggregator/force_display_results/+merge/267156
> You are the owner of lp:~gary-wzl77/scope-aggregator/force_display_results.
>

--
Br
Gary.Wzl

« Back to merge proposal