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

DTM-13100 Report rule completion when rule.actions is undefined. #85

Merged
merged 3 commits into from Apr 16, 2019

Conversation

Aaronius
Copy link
Contributor

Currently, if a Launch library container has a rule that does not have an actions property defined and the rule executes, two things fail to happen:

There is no log message emitted to the console saying that the rule completed.
Launch monitoring hooks don't notify monitors that the rule completed.
This particularly affects users of the Sandbox when they're developing an event type. If they use the library sandbox editor to create a rule that uses their event type, but they don't add an action to the rule (currently there's no built-in action type provided by the sandbox that they could use), there's no way of knowing if their event type is actually running rules like they would expect, because no log message ever shows up saying the rule was run.

@Aaronius Aaronius requested a review from dompuiu April 16, 2019 14:45
@coveralls
Copy link

coveralls commented Apr 16, 2019

Coverage Status

Coverage increased (+0.1%) to 98.297% when pulling 0238a3a on actions-undefined-report-completion into 09c907a on master.

@dompuiu
Copy link
Member

dompuiu commented Apr 16, 2019

Should we add a test for this?

@Aaronius
Copy link
Contributor Author

Yes. I've added tests.

@dompuiu
Copy link
Member

dompuiu commented Apr 16, 2019

Looks good.

@Aaronius Aaronius merged commit cece104 into master Apr 16, 2019
@Aaronius Aaronius deleted the actions-undefined-report-completion branch April 16, 2019 19:20
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.

None yet

3 participants