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

CronTime day of week is non-standard (Sunday is 1, not 0) #36

Closed
danhbear opened this issue Jun 5, 2012 · 10 comments
Closed

CronTime day of week is non-standard (Sunday is 1, not 0) #36

danhbear opened this issue Jun 5, 2012 · 10 comments

Comments

@danhbear
Copy link
Contributor

danhbear commented Jun 5, 2012

The documentation referenced in the readme says that day of week is 1-7, Sunday being 1.
http://help.sap.com/saphelp_xmii120/helpdata/en/44/89a17188cc6fb5e10000000a155369/content.htm

All other crontab documentation I see says that day of the week is 0-6, Sunday being 0.
http://crontab.org/
http://en.wikipedia.org/wiki/Cron
http://www.adminschoice.com/crontab-quick-reference

Mind if I submit a patch to change? Or would you like a "compatibility mode" that explicitly requests real cron times?

I'd also like to support the standard 5-digit crontime syntax that doesn't include seconds. Mind if I submit a patch to default to "a b c d e f" to "0 a b c d e f" ?

@ncb000gt
Copy link
Member

ncb000gt commented Jun 8, 2012

Hey,

Yes. I speak patches. :) I would like things to be consistent. Obviously both would break things so I'd upgrade the version and make it clear that there was a break in backwards compatibility. But, I'm ok with this so long as it makes the library more consistent.

@pixelfreak
Copy link

Hello,

When is this getting into the code? Currently, it's definitely inconsistent.

Thanks!

@ncb000gt
Copy link
Member

I haven't seen a patch for this yet. I'm not sure if anyone is actively working on it.

@danhbear
Copy link
Contributor Author

I can pick this up again at some point. @pixelfreak, are you more interested in the 5-digit crontime syntax or the day-of-week inconsistency?

@pixelfreak
Copy link

More interested in the day-of-week inconsistency. Thanks!

@pixelfreak
Copy link

Actually, I am curious why there exists two different cron format and which is better. For example, the linux cron we are all familiar with can't do seconds interval, but this one can. So maybe it's better to keep it that way?

@danhbear
Copy link
Contributor Author

Since linux cron is the standard, personally I think node-cron should be consistent with its syntax/functionality. This would mean changing day-of-week to be consistent. Since the seconds interval doesn't contradict any standard cron syntax, it should just be an optional extended syntax (only if 6 digits are included). Will prepare the patch and see if people need backwards compatibility.

@ncb000gt
Copy link
Member

Sounds good. I'll review it in a separate branch anyway before including it.
On Jul 23, 2012 10:05 PM, "Daniel Bear" <
reply@reply.github.com>
wrote:

Since linux cron is the standard, personally I think node-cron should be
consistent with its syntax/functionality. This would mean changing
day-of-week to be consistent. Since the seconds interval doesn't contradict
any standard cron syntax, it should just be an optional extended syntax
(only if 6 digits are included). Will prepare the patch and see if people
need backwards compatibility.


Reply to this email directly or view it on GitHub:
https://github.com/ncb000gt/node-cron/issues/36#issuecomment-7196981

@danhbear
Copy link
Contributor Author

Alrighty, pull request up at https://github.com/ncb000gt/node-cron/pull/41. There are some comments in there. Let me know what you think.

@ncb000gt
Copy link
Member

ncb000gt commented Aug 7, 2012

Completed with GH-41.

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

No branches or pull requests

3 participants