Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function to show public holiday result #159

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

KevCui
Copy link

@KevCui KevCui commented Feb 28, 2021

No description provided.

@sudocanttype
Copy link
Contributor

sudocanttype commented Mar 1, 2021

Nice. Can you write some test cases for it and send the testoutputs.txt? Or if you don't want to, I can make a new PR with the testcases.

@sudocanttype
Copy link
Contributor

K I made a pr to your main for testing. @KevCui

added testing for public holidays
@KevCui
Copy link
Author

KevCui commented Mar 1, 2021

Thanks @sudocanttype 👍

@sudocanttype
Copy link
Contributor

output.txt
Proof that it works. If someone else wants to test before we merge in, go for it

@sudocanttype
Copy link
Contributor

@BeyondMagic Can you test this branch and see if it works for you? If it does, i think this is ready to merge

@BeyondMagic
Copy link
Contributor

I think Basic Answers has conflict with Public Holidays, Public Holidays should be put before it.

@sudocanttype
Copy link
Contributor

Can you give me an example of where it conflicts? I need something to test to fix the problem

@BeyondMagic
Copy link
Contributor

try "christmas" with both features

@sudocanttype
Copy link
Contributor

oh yea, i see it

@BeyondMagic
Copy link
Contributor

BeyondMagic commented Mar 3, 2021

also, I think we can add a better sed there, like

holiday's name
* holiday's day
holiday's name
* holiday's day
...

or

holiday's name - holiday's day
holiday's name - holiday's day
holiday's name - holiday's day
...

@sudocanttype
Copy link
Contributor

what exactly is ./tuxi christmas supposed to return? are we trying to give the definition?

@BeyondMagic
Copy link
Contributor

I mean, it should, but it's not.

@sudocanttype
Copy link
Contributor

This doesnt seem to be a problem with the public holiday function. Try checking out the main branch of tuxi and trying ./tuxi christmas. For me it still returns the same thing, so something else is messing with it

@BeyondMagic
Copy link
Contributor

Oh yeah, I remember fixing this in develop branch, I thought this would fix this too in the main since the name is for public holidays

@sudocanttype
Copy link
Contributor

How did you fix it in the develop branch?

@BeyondMagic
Copy link
Contributor

by adding -a to see all results, lol

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

Successfully merging this pull request may close these issues.

3 participants