search

duckduckgo / zeroclickinfo-spice

DuckDuckGo Instant Answers based on JavaScript (JSON) APIs
https://duckduckhack.com/
Other
548 stars 942 forks source link

Currency Suggestion: Add $ and £ in currency names to expand the coverage for currency spice #3379

Closed adityatandon007 closed 7 years ago

adityatandon007 commented 7 years ago

I wasn't going to create this issue but couldn't help myself. I thought I would directly send the PR and I thought it was going to be simple to correct this bug.

Bug: Queries like 2k us$ in inr or 2k gb£ in inr triggers in live version but gives incorrect answer but don't trigger in beta/duckpan. I think this happens because of that special triggering in DDG which @pjhampton mentioned in https://github.com/duckduckgo/zeroclickinfo-spice/issues/3372#issuecomment-317416342 So, I thought I will remove this bug by adding aliases like us$ and gb£ in their Currency Names and then I thought I should add this functionality to every other currency like au$ for Australian dollar. I made the PR and pushed it (on my repo) and then before making the request I thought lets give it a try in duckpan but to the surprise it didn't work. I thought of looking into the problem but couldn't figure out what could be the problem, then I noticed something on duckpan that it is making a request like this 2k us%24 in inr. So what I could get is that it is encoding my $ but when we make request on live version that $ is not encoded? although gives the wrong answer. Here are the screen shots

screenshot from 2017-07-26 15-38-51 screenshot from 2017-07-26 16-17-27

People to notify @pjhampton @mintsoft

PS. I have the PR ready but it doesn't work. Should I push it?

IA Page: http://duck.co/ia/view/currency Maintainer: @pjhampton

adityatandon007 commented 7 years ago

@pjhampton let me know if I should push my PR

boozook commented 7 years ago

Then ruble sign should be here too.

₽

pjhampton commented 7 years ago

I delayed my reply while I thought about this in depth. I don't disagree with you guys, but...

The purpose of writing and deploying the special internal triggering code was to cover any edge cases. This would prevent me and staff from playing cat and mouse with them. My definition of edge case is not frequently searched for, and nothing urgent. I think this definition should propagate back throughout the community.

Apart from a well needed refactor (perl backend), I would say this IA is stable. It doesn't need much more work to meet the expectations/wants of DuckDuckGo users. Saying that I think what would be best is that you guys (as a community):

  1. Push unsolicited PRs to patch triggering edge cases (that will be reviewed and merged ~1-5 weeks by a comm leader or staff) without opening issues for every edge case.
  2. Close this and maintain a master issue where community members such as yourselves drop edge cases, and a nominated community dev send patches for these.

I think you guys (the community) should get inventive and help keep making these stable IAs even better - without bikeshedding. We should aim to work on the stuff that has the most impact for the end user. That is what makes an open source developer a great developer - impact.

adityatandon007 commented 7 years ago

@fzzr- the reason why I suggested only $ and £ because these are easily available as keys in keyboards so any user can them with ease. Although I know very few people (even less than 0.0001%) will type something like us$ or gb£ but since duckduckgo is programmer friendly search engine, I thought we should support these kinds of tricker queries too. I don't see any point to not add ₽ sign to support Russian ruble except very few keyboards have ₽ as keys. One more thing after reading your comment I tried some queries like 100 $ to ₽ and expected the currency IA to trigger because we support symbols too but it didn't and then I tried something like 100 $ to £ and voilà it triggered and showed the right answer as I was expecting. I don't know the reason for this but I will open up a separate issue for this.

adityatandon007 commented 7 years ago

@pjhampton I didn't get the reason for closing that issue. If you want to include that issue to be taken care here, then thats fine

pjhampton commented 7 years ago

Yes - that's what I'm trying to say. Stop creating issues for edge cases. Just push PRs and let them fall into a review cycle.

pjhampton commented 7 years ago

Hey @adityatandon007!

DuckDuckHack is now in Maintenance Mode and from now on, we are only accepting issues and PRs for essential bugs and bug fixes.

Unfortunately, with the above in mind, this isn't something we can action and will be closed.

We appreciate you taking the time to contribute and apologize for not being able to triage this issue.