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 possibility to use question marks in days and weekdays #3

Merged

Conversation

miboz
Copy link
Contributor

@miboz miboz commented Nov 29, 2019

No description provided.

@GuillaumeRochat
Copy link
Owner

Thanks for the contribution @miboz!

Since this appears to be a non-standard character, I think it should be behind a flag, similarly to alias and seconds. Not all systems would accept it I believe. https://en.wikipedia.org/wiki/Cron#Non-standard_characters

What do you think? If so, how could it be named?

@miboz
Copy link
Contributor Author

miboz commented Dec 2, 2019

Thank you for your quick response @GuillaumeRochat ! Yes, you are totally right, I think it should be a flag too. As of the name of the flag, I am not sure what would be the best fit. Maybe something like "optional-day-format" or simply "question-mark" ?

@GuillaumeRochat
Copy link
Owner

@miboz A quick search only shows up question mark as the way to name it, but it's described as an Either/Or for either the day of month or the day of week.

We can make it simple, use questionMark, and add a good enough description in the readme about the flag.

The downside of calling it just questionMark though is that there seem to be another use according to the wikipedia article. But that other feature could have its own name if it ever gets added (maybe probably never).

Sources :
https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/ScheduledEvents.html
https://en.wikipedia.org/wiki/Cron#Non-standard_characters

@miboz
Copy link
Contributor Author

miboz commented Dec 3, 2019

@GuillaumeRochat Indeed, I agree, questionMark is not a perfect solution, and it would be nice to avoid using it if possible.

As for the name of the flag that should be used, I must admit that I do not have many ideas. Here are the few ideas that I had: optionalDayFormat or questionMarkDayFormat

And do not hesitate to suggest flag names if you think of anything!

@GuillaumeRochat
Copy link
Owner

@miboz How about allowBlankDay? In the wikipedia description, they say

used instead of '*' for leaving either day-of-month or day-of-week blank.

@miboz
Copy link
Contributor Author

miboz commented Dec 3, 2019

@GuillaumeRochat It sounds like a good name to me! It corresponds the best to the wikipedia definition.

@GuillaumeRochat
Copy link
Owner

@miboz Good! I'll review again once you added the flag checks. If you need any more inputs for doing so, let me know.

@miboz
Copy link
Contributor Author

miboz commented Dec 3, 2019

@GuillaumeRochat Perfect! Thank you for everything!

Copy link
Owner

@GuillaumeRochat GuillaumeRochat left a comment

Choose a reason for hiding this comment

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

Everything else looks perfect. 👍

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@GuillaumeRochat
Copy link
Owner

@miboz Oh also, add an example in the readme! I almost forgot about that part.

@GuillaumeRochat GuillaumeRochat merged commit 202f389 into GuillaumeRochat:master Dec 4, 2019
@GuillaumeRochat
Copy link
Owner

Thanks again for the contribution! Really appreciated.

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.

2 participants