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

Patch for issue #41 #42

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

dalgard
Copy link

@dalgard dalgard commented Feb 23, 2015

#41

@zol
Copy link
Member

zol commented Feb 23, 2015

Thanks for you patch @dalgard . Before I'm able to merge it, could you please add some tests around it?

@dalgard
Copy link
Author

dalgard commented Feb 23, 2015

Can you see my two latest commits under this pull request?

I don't know how to write the code around the call to SyncedCron.add inside the Tinytests. This is as good as I can make it. Also added documentation. Thanks :)

@zol
Copy link
Member

zol commented Feb 23, 2015

You could take a look at synced-cron-tests.js for inspiration. It will take me some time to get around to writing tests against this PR. We have a policy of not merging untested code into our packages.

@dalgard
Copy link
Author

dalgard commented Mar 16, 2015

Sorry for the late reply. It's a sound attitude, but this library doesn't have the greatest test coverage to begin with and the change is very little – I don't know how to write Tinytests, but with my scaffolding, it will take you very little time.

At any rate, I'm using the patch in production – I really think you should consider it :) Thanks!

…nced-cron

Conflicts:
	synced-cron-server.js
	synced-cron-tests.js
@tylerstraub
Copy link

In attempting to use the parser.cron method with this forked version, I am experiencing recursive calls during the execution stage of a scheduled job.

With a CRON syntax input such as "58 13 30 4 ? 2015", I am experiencing these results:

SyncedCron.add({
  name: 'Crunch some important numbers for the marketing department',
  schedule: function(parser) {
    // parser is a later.parse object
    return parser.cron('58 13 30 4 ? 2015');
  },
  job: function() {

 /* Anything placed here is firing recursively */

  }
});

What methods for parsing the later.js object have you been successful with in your fork @dalgard?

Thanks!

@dalgard
Copy link
Author

dalgard commented May 5, 2015

I don't really know the cron syntax, I'm using this line in my application:

// Non-recurring
return parser.recur().on(ending_at).fullDate();

@StorytellerCZ StorytellerCZ linked an issue Sep 28, 2023 that may be closed by this pull request
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 servers can still run the same job at the same time
3 participants