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

Re-work :skip_first_run and :at interaction #22

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

Conversation

ChaoticBoredom
Copy link

@ChaoticBoredom ChaoticBoredom commented Oct 4, 2017

As @MarcLapierre pointed out here the skip first run option broke cases where multiple :at were used. This was due to myself not truly understanding how :at worked. It will actually schedule an event for each :at argument, and each of those events would then have :skip_first_run causing it to skip a single first run. W/ a long enough period and short enough restart interval it would cause these events to never run.
The change to make this work properly is to move the :skip_first_run checks from out of the event and into the manager. When the first event is run, it will skip, and then look for any identical events and clear the skip_first_run flag on all of those. So the first event will be skipped, but subsequent events should run perfectly fine.

My apologies for missing the concern, saw the first comment about it breaking the test gem, but then didn't read about the failure of the interaction.

One thought about this, it might be worthwhile to be able to gather these events via some unique ID, instead of relying on users naming their events uniquely.

@pboling
Copy link

pboling commented Oct 4, 2017

it might be worthwhile to be able to gather these events via some unique ID, instead of relying on users naming their events uniquely.

Alternatively we could have it raise an error if events with the same name are detected from subsequent definitions, by maintaining a unique registry populated as the DSL is invoked to define the events.

@ChaoticBoredom
Copy link
Author

it might be worthwhile to be able to gather these events via some unique ID, instead of relying on users naming their events uniquely.

Alternatively we could have it raise an error if events with the same name are detected from subsequent definitions, by maintaining a unique registry populated as the DSL is invoked to define the events.

Yup, either or works for me. Just wanted to call that out. If it's a big enough issue I can work on fixing that here as well.

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