Add every: Proc support + until: val + until: Proc support

[jon-sully]

Currently possible:

periodical :check_in, every: 2.weeks, start: -> { 5.hours }

With somewhat-bug described here in that the start value is re-assessed with every period and causes drift

Breaking Changes

start: no longer drifts the periods, it's only used for the first message in the sequence

Now possible:

periodical :check_in, every: -> { 2.weeks + rand(0..3).days + rand(0..100).minutes }, start: -> { subscriber.time_until_messageable }
# or 
periodical :check_in, every: -> { subscriber.check_in_frequency }, until: -> { subscriber.no_longer_wants_check_ins }
# or
periodical :check_in, every: 2.weeks, until: "01/01/2020"

But we should be careful with the last one (static until: value) — setting an until: with a computed date will get re-computed each time the Rails app process boots up! It's available for those that need it, but with caution! If it always gets recomputed, it'll never actually stop messaging 😛

I'll need to update docs before this is ready as a PR.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.34 ⚠️

Comparison is base (e9bcc5a) 99.08% compared to head (46e47f4) 98.74%.

❗ Current head 46e47f4 differs from pull request most recent head 087e544. Consider uploading reports for the commit 087e544 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
- Coverage   99.08%   98.74%   -0.34%     
==========================================
  Files          83       83              
  Lines        2069     2149      +80     
==========================================
+ Hits         2050     2122      +72     
- Misses         19       27       +8     

Impacted Files	Coverage Δ
lib/caffeinate/drip.rb	90.90% <100.00%> (-4.10%)	⬇️
lib/caffeinate/dripper/drip_collection.rb	100.00% <100.00%> (ø)
lib/caffeinate/dripper/periodical.rb	100.00% <100.00%> (ø)
lib/caffeinate/schedule_evaluator.rb	98.18% <100.00%> (+0.56%)	⬆️
spec/caffeinate/deliver_async_spec.rb	100.00% <100.00%> (ø)
spec/caffeinate/dripper/cases/callbacks_spec.rb	99.22% <100.00%> (+<0.01%)	⬆️
spec/caffeinate/dripper/cases/periodical_spec.rb	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[joshmn]

I wonder if it makes sense to validate separate keys for periodical? subclass Drip? Spitballing here, not a prescription.

[jon-sully]

I could totally see that. Actually the separation between drip and periodical totally isn't clear right now and may well be a foot-gun as is since you can pass every: (and now until:) and start: to drip and it will work for the first mailing, but the subsequent mailings won't get re-enqueued! So you have to use periodical, but I think that's both not-currently documented and a super-hazard that's easy to do. We were about to use drip .. every: 2.weeks before realizing the subsequent-repeating wouldn't have fired if we did.

[joshmn]

I'll push it up in a sec; might need some help documenting it... hehe

[joshmn]

can you help with #26? i think once that's merged this might be more tenable?

[jon-sully]

Yeah!

[joshmn]

one thing about this:

keyword arg until will throw a fit cuz it's reserved. :)

perhaps if?

[jon-sully]

perhaps if?

We'd have the same problem. The keyword issue is that we're passing the arg into a hash and it would look like this:

        def periodical(action_name, every:, start: -> { ::Caffeinate.config.time_now }, until: -> { 5_000.years.from_now }**options, &block)
           options[:start] = start
           options[:every] = every
           options[:until] = until

           drip(action_name, options, &block)

           # etc.

Even Github's Ruby parser marks the until word there ^ red 😆 can't use until as an argument name. if would have the same problem. So I just didn't break it out from options and set it with ||= instead:

         def periodical(action_name, every:, start: -> { ::Caffeinate.config.time_now }, **options, &block)
           options[:start] = start
           options[:every] = every
           options[:until] ||= 5_000.years.from_now # can't call this out as a param above because `until` is a Ruby keyword

           drip(action_name, options, &block)

           # etc.

But outside of Ruby language logistics (and the trouble that comes with either until or if) the question is, which term presents a more readily-understandable and simple interface?

"Do this every X days, starting Y, until Z"

or

"Do this every X days, starting Y, if Z"

[jon-sully]

"Do this every X days, starting Y, until Z"

or

"Do this every X days, starting Y, if Z"

I think between these two I'm still on team until 🤔

[joshmn]

(perhaps if was a really bad attempt at a joke)

do we want to merge #26 first?

[jon-sully]

(perhaps if was a really bad attempt at a joke)

🤣 then lol at my totally not getting that and further lol at my explanation

Yeah, no rush here; I'm in favor of getting #26 cleared beforehand. I should have time tomorrow to look through #24 and #26 more fully!

[jon-sully]

@joshmn any interest in this PR still? Happy to rebase; I think master still has a couple of hiccups / bugs around start only being used in the first iteration of a periodical — hope to iron that out even if we don't end up going with :until

[jon-sully]

Note to self: I should come back to this, and maybe instead of having until: <proc> we use while: <PROC> key, since "while" naturally implies a "while this is true" API, which makes simpler cognitive sense than "until this is true? until this is false?"

[jon-sully]

I don't think this PR is needed anymore. See extensive writeup at #41