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

Add flags to trackEvent API #861

Merged
merged 3 commits into from
Nov 2, 2018
Merged

Conversation

guperrot
Copy link
Member

@guperrot guperrot commented Nov 1, 2018

Please have a look at our guidelines for contributions and consider the following before you submit the PR:

  • Has CHANGELOG.md been updated?
  • Are tests passing locally?
  • Are the files formatted correctly?
  • Did you add unit tests?
  • Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?

Description

Add flags to trackEvent that take EventProperties as a parameter.
Use Flags.DEFAULT_FLAGS for all previous signatures.

* @param flags Optional flags. Use {@link Flags#PERSISTENCE_CRITICAL} to send this event
* before events using that use default flags or {@link Flags#PERSISTENCE_NORMAL}.
*/
public static void trackEvent(String name, EventProperties properties, int flags) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Something like @IntRange doesn't apply here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't use them for public APIs as it triggers warnings for people not having support annotations. So we validate and log at runtime everywhere in the SDK.

@guperrot guperrot changed the title Work in progress for trackEvent with flags Add flags to trackEvent API Nov 2, 2018
@guperrot guperrot changed the base branch from feature/send-priority to feature/priority November 2, 2018 01:26
Copy link
Contributor

@jaeklim jaeklim left a comment

Choose a reason for hiding this comment

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

I'm approving this PR but haven't tested yet. The test will be done with test app implementation.

@guperrot guperrot merged commit d1a6568 into feature/priority Nov 2, 2018
@guperrot guperrot deleted the feature/track-event-flags branch November 2, 2018 18:55
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.

3 participants