Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Bike Sharing Philadelphia (Indego): Add triggers for official name #2878

Closed
laurenancona opened this issue Jul 5, 2016 · 5 comments
Closed

Comments

@laurenancona
Copy link
Collaborator

I'm not super familiar with the way triggering works with places - whether any permuting of keywords + place triggers is happening under the hood or not - either way, this isn't triggering on indego bike share/ing (official name of the program).

Happy to PR, but wanted feedback to avoid keyword redundancy.


IA Page: http://duck.co/ia/view/bike_sharing_indego_phl
Maintainer: @AcriCAA

@tagawa
Copy link
Collaborator

tagawa commented Jul 30, 2016

@laurenancona Thanks for reporting - I agree this should be triggering as you point out.

At the moment, there are two stages:

  1. Initial triggers to execute the backend Perl code
  2. A check for place names (@places) to execute the frontend JS code

The triggers themselves are fine — indego bike share would pass — so it probably needs a new check for the "indego" brand name but should also include "bike" or "ride", to exclude unrelated "indego" queries.

In psuedo-code, that might be something like:

return unless (place is valid and in @places array) OR (query includes "indego" and ("bike" or "ride"));

Alternatively, a more readable form could be:

if (place is valid and in @places array) {
    return '';
} else if (query includes "indego" and ("bike" or "ride")) {
    return '';
}

That's the best solution I can come up with. If you'd like to make a PR I'm sure we can get some eyes on it.

@AcriCAA
Copy link
Contributor

AcriCAA commented Jul 31, 2016

I'll have to look back at the code but, as I recall, the bikeshare search
functionality is location based since this is not the only city for which
bikeshare info is published on DDG. So an indication of the city in some
way is the first trigger, then the bikeshare trigger, which could be the
brand name. That's why something like "philly indego" and "Philadelphia
indego" will work. It could also be the reverse. But, yeah, it could easily
be revised. This is just the best we came up with at the time since we were
working on a multi-city approach. But iterate away!
On Sat, Jul 30, 2016 at 2:52 AM Daniel Davis notifications@github.com
wrote:

@laurenancona https://github.com/laurenancona Thanks for reporting - I
agree this should be triggering as you point out.

At the moment, there are two stages:

  1. Initial triggers to execute the backend Perl code
  2. A check for place names (@places) to execute the frontend JS code

The triggers themselves are fine — indego bike share would pass — so it
probably needs a new check for the "indego" brand name but should also
include "bike" or "ride", to exclude unrelated "indego" queries.

In psuedo-code, that might be something like:

return unless (place is valid and in @places array) OR (query includes
"indego" and ("bike" or "ride"));

Alternatively, a more readable form could be:

if (place is valid and in @places array) {
return '';
} else if (query includes "indego" and ("bike" or "ride")) {
return '';
}

That's the best solution I can come up with. If you'd like to make a PR
I'm sure we can get some eyes on it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2878 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEgw1HbytnaCM4VcwU952tw8V7qUcn2eks5qavSdgaJpZM4JEyIc
.

@laurenancona
Copy link
Collaborator Author

Hi @AcriCAA!

Thanks all, I'll take a look.

@AcriCAA
Copy link
Contributor

AcriCAA commented Aug 1, 2016

Hey @laurenancona! I took a quick look at the code and this is the line I
was referring to in my last comment:

https://github.com/duckduckgo/zeroclickinfo-spice/blob/master/lib/DDG/Spice/BikeSharing/IndegoPHL.pm#L11

One of the triggers is the place. It's what separates DDG from returning
the philly bikeshare info from other cities. I suppose, an easy and
probably not so appropriate fix would be to just add "indego" as a "place."
Otherwise, a rewrite may be necessary, but if you take that approach, we
might want to make sure the other bikeshare cities behave the same way.

On Sun, Jul 31, 2016 at 2:53 PM, Lauren Ancona notifications@github.com
wrote:

Hi @AcriCAA https://github.com/AcriCAA!

Thanks all, I'll take a look.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2878 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEgw1JgXEbBYhHQBBND4zvEIbscHL8oQks5qbO8MgaJpZM4JEyIc
.

@moollaza
Copy link
Member

Closing due to DDH entering Maintenance Mode. We are now only accepting issues for essential bugs/fixes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants