Addition of some popular features

[dennisfrancis]

1. Seconds field is now optional and Dow field is mandatory to support standard GNU/Linux crontab format.

2. An optional TZ= clause (ex : TZ=Europe/London) in the beginning of cron spec to support specification of cron times in that location rather than in the local timezone

Sample usage :
c := cron.New()
c.AddFunc("TZ=Europe/London 07 21 * * *", func() { fmt.Println("Runs at 21:07 London time") }, "London test")
c.Start()

3. Added a function NextNTimes for Entry struct which returns next (upto) N activation times.

4. Added DST support and test cases from github.com/lukescott/cron feature-dst branch

5. Added safe Remove() and its test cases.

[unknwon]

+1

[robfig]

This looks really good, thank you. Wanted to discuss two items before getting into the nitty gritty:

• I'm not convinced that adding the Text member is worthwhile. Callers can easily make a Job implementation that has it (or, we could provide one , e.g. NamedJob), and the cron framework does not make use of it. If possible, I'd like to limit backwards incompatible changes
• How do you feel about returning the *Entry , and having the caller use that as the key to Remove? That could avoid introducing a new type and member

Thanks again for your good work! I know I'm not a very responsive maintainer, but I'll try to keep on this

[dennisfrancis]

Thanks.

1. Good point, Text can live inside user implementation of Job interface. I will remove the Text field so that the exported functions are backward compatible.

2. I feel that EntryId has two advantages against using a client visible *Entry as Id:
   -- Having a EntryId type makes it harder for the users to edit a live(when cron is running) Entry directly by accessing its exported members (like Next for example) which is unsafe.
   -- In future we can always change the implementation of the actual Id inside EntryId without breaking the client code. (Right now it is only *Entry).

[robfig]

Regarding 2.. it seems like providing the Entry back to clients would be helpful so they can access the members -- them having access to the Next field doesn't seem like that big of a problem to me, and they can already get it by calling Entries()

My biggest concern about this changeset is that it changes the meaning of 5-element cron specs, which would invisibly break clients. I wonder how we could avoid that..

[dennisfrancis]

Entries() returns only a snapshot(copy) of the actual Entries (Cron.entries). Modifying Entry from Entries() will not affect the Cron.entries. If Entry from Cron.entries were available to the user directly, it is possible to skip job runs.

[robfig]

It looks like it may be best to handle the backwards incompatibility using gopkg.in

So, importing github.com/robfig/cron can be equivalent to gopkg.in/robfig/cron.v1 , and this collection of new features can go into the v2 branch, which is imported at gopkg.in/robfig/cron.v2. That means I'll leave master at v1

Sounds ok?

[dennisfrancis]

Yes, Thanks.
Looking forward to see all of the 5 features in v2 branch.

[robfig]

Yeah, you're right that the Entry ID approach is the right one. The thing that pushed me over was that cron needs concurrent write access to Next and Prev. I also made the snapshots into values, to make it clear that they are copies and not pointers to the original.

Seems ok?
2b666ea

[robfig]

The "DST support" piece didn't work against the larger test suite. For example, it would run a 2am nightly job twice during a 2am -> 1am transition, although it does seem to fix some of the other cases. It may take me a while to figure it out. Seems complicated

[dennisfrancis]

Yes, that makes it clear for the user.

[abecciu]

All these changes are really great. I'd love to see them merged. Thanks!

[robfig]

Closing dead PR