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

Add IsItChristmas IA #1079

Closed
wants to merge 7 commits into from
Closed

Add IsItChristmas IA #1079

wants to merge 7 commits into from

Conversation

stevenmg
Copy link

IsItChristmas

What does your Instant Answer do?
It adds a simple and fun answer to the question of whether or not it is currently Christmas.

What problem does your Instant Answer solve (Why is it better than organic links)?
It allows people to find an answer to the important question of whether or not it is Christmas without the need to click through to isitchristmas.com.

What is the data source for your Instant Answer? (Provide a link if possible)
The current Date combined with my knowledge of the fact that Christmas is always December 25th

Why did you choose this data source?
This question seems invalid in the current context.

Are there any other alternative (better) data sources?
Nope.

What are some example queries that trigger this Instant Answer?
"is it christmas"

Which communities will this Instant Answer be especially useful for? (gamers, book lovers, etc)
Anyone who loves Christmas

Is this Instant Answer connected to a DuckDuckHack Instant Answer idea?
Nope

Which existing Instant Answers will this one supercede/overlap with?
None

Are you having any problems? Do you need our help with anything?
Using Codio it is impossible to change the date in order to ensure that the IA really will say "Yes" on December 25th so if someone could test that more thoroughly that would be great.

What does the Instant Answer look like? (Provide a screenshot for new or updated Instant Answers)
Is It Christmas?

Checklist

Please place an 'X' where appropriate.

- [x] Added metadata and attribution information (https://duck.co/duckduckhack/metadata)
- [x] Wrote test file and added to t/ directory (https://duck.co/duckduckhack/goodie_tests)
- [x] Verified that instant answer adheres to design guidelines (https://duck.co/duckduckhack/design_styleguide)
- [] Verified that instant answer adheres to code styleguide (https://duck.co/duckduckhack/code_styleguide)
- [] Tested cross-browser compatibility

    Please let us know which browsers/devices you've tested on:
    - Windows 8
        - [] Google Chrome
        - [x] Firefox
        - [] Opera
        - [] IE 10

    - Windows 7
        - [] Google Chrome
        - [] Firefox
        - [] Opera
        - [] IE 8
        - [] IE 9
        - [] IE 10

    - Windows XP
        - [] IE 7
        - [] IE 8

    - Mac OSX
        - [] Google Chrome
        - [] Firefox
        - [] Opera
        - [] Safari

    - iOS (iPhone)
        - [] Safari Mobile
        - [] Google Chrome
        - [] Opera

    - iOS (iPad)
        - [] Safari Mobile
        - [] Google Chrome
        - [] Opera

    - Android
        - [] Firefox
        - [] Native Browser
        - [] Google Chrome
        - [] Opera

@moollaza
Copy link
Member

@stevenmg thanks for contributing! We really appreciate it 😃

We'll be sure to give you some feedback shortly!

print($m);
print($d);

if ($m == 12 && $d == 25) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that ($m == 12 && $d == 25) ? return "Yes" : return "No"; is styled better ;)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would disagree! I'd much prefer

return "Yes" if ($m == 12 && $d == 25);
return "No";

or

return "No" unless ($m == 12 && $d == 25);
return "Yes";

@puskin94
Copy link
Collaborator

@mintsoft only as a comparison... Why do you prefer your if solution instead of mine?

@mintsoft
Copy link
Collaborator

@puskin94 A few relatively minor reasons; it's more certainly more perlistic. The main advantage is that I think it's clearer to see the default vs the exception case whereas with a ternary it's nowhere near as obvious imo

@puskin94
Copy link
Collaborator

@mintsoft Yep, it is more and more explicit. But, in my opinion and in the style guide in my own mind, I think that compact is better ;)

@mintsoft
Copy link
Collaborator

@puskin94 I think it's probably a matter of style mainly. In my experience I've found that clarity is the most important thing, especially when working in a team; however I'm sure there are other philosophies.

@stevenmg
Copy link
Author

Thanks for the input guys. I typically prefer more clear over more compact. Does the style guide specify which one should be used?

Also, I like the idea of using mock time for tests, I will work on that later.

@mintsoft
Copy link
Collaborator

@stevenmg it's totally up to you; whichever you prefer!

@mintsoft
Copy link
Collaborator

@stevenmg also btw I don't see any real problem with what you have currently there; there's certainly no need to change it unless you want to

@mintsoft
Copy link
Collaborator

mintsoft commented Apr 3, 2015

@stevenmg I've added the Work in Progress label for now, whenever you're finished up just remove it 👍

@stevenmg
Copy link
Author

stevenmg commented Apr 5, 2015

My mental style guide says to keep it as is (probably because I use Java a lot) so I think that is what I'll do unless there is another reason to change it.

@stevenmg
Copy link
Author

stevenmg commented Apr 5, 2015

I have added mock time for tests as well as 'is it xmas' as a trigger phrase. If anything else should be added let me know.

@stevenmg
Copy link
Author

stevenmg commented Apr 5, 2015

I was just looking at the Travis CI status for my earlier commits and it looks like the tests are failing for this IA. I am going to wait for the results of the latest commits and then see what I can do.

...or I'll just run a test from Codio :)

@stevenmg
Copy link
Author

stevenmg commented Apr 5, 2015

I just fixed up the test file. I think I'm done here.

# ABSTRACT: Answers the question of whether or not it is Christmas with a simple yes or no answer

use DDG::Goodie;
use Date::Calc;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you switch this to: use DateTime; as we already use this module in the Dates role

@javathunderman
Copy link
Collaborator

@mintsoft @stevenmg Is this ready to be merged?

@mintsoft
Copy link
Collaborator

@javathunderman almost, @abeyang does this look aright to you or would you prefer it to use the structured answer?

@javathunderman
Copy link
Collaborator

ping @mintsoft @abeyang

@moollaza
Copy link
Member

Hey guys, we're discussing this IA internally with regards to whether or not it deserves to be an IA given the narowness of the queryspace. We may decide it needs to be generalized, or perhaps this should be handled by the coming Holidays IA.

I'll be sure to keep you updated!

@javathunderman
Copy link
Collaborator

Holidays IA? :)

@moollaza
Copy link
Member

@moollaza
Copy link
Member

Hey guys, sorry for the long delay. I feel like the queries this IA handles should really be handled by the coming Holidays IA as it has a much broader scope.

We feel that this IA is a little too narrow in scope.

@stevenmg I'm really sorry we can't move forward with this but we really hope you're still interested in contributing! Is there anything else you'd like to work on? A cheat sheet perhaps?

@moollaza moollaza closed this Jul 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants