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

Make all callbacks return boolean values #534

Merged
merged 4 commits into from
Apr 16, 2020
Merged

Make all callbacks return boolean values #534

merged 4 commits into from
Apr 16, 2020

Conversation

fractalwrench
Copy link
Contributor

Goal

Updates the OnSendBlock and OnSessionBlock callbacks to return boolean values which control whether the event/session is sent or not.

Changeset

  • Updated return type of blocks to BOOL for both callback types
  • Moved OnSessionBlock callbacks into the SessionTracker. If a callback returns false, the new session is discarded and the original session (if any) is used

Note: the session callback is effectively passed an empty dictionary as its parameter for now, which has been noted as a TODO comment. Future work has been scheduled which will alter the parameter to take a structured class with session information instead.

Tests

Added new unit tests to verify the behaviour for session blocks.

@fractalwrench fractalwrench changed the title Make all callbacks return boolean Make all callbacks return boolean values Apr 14, 2020
Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor nits, but looks good otherwise.

Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fractalwrench fractalwrench merged commit f7cb9cc into v6 Apr 16, 2020
@fractalwrench fractalwrench deleted the v6-blocks branch April 16, 2020 09:19
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