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

remove initializer for setting identifiers in events #134

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

ndurell
Copy link
Contributor

@ndurell ndurell commented Jan 4, 2024

Description

We no longer want to allow setting identifiers, time, and profile in events. This can create confusing bugs for developers and brings it in line with the Android SDK.

Check List

  • Are you changing anything with the public API?
  • Have you tested this change on real device?
  • Are your changes backwards compatible with previous SDK Versions?
  • Have you added unit test coverage for your changes?
  • Have you verified that your changes are compatible with all the operating system version this SDK currently supports?

Manual Test Plan

Supporting Materials

@ndurell
Copy link
Contributor Author

ndurell commented Jan 4, 2024

This will be released as a minor point release first and then removed in upcoming 3.0.0.

@ndurell ndurell marked this pull request as ready for review January 4, 2024 18:43
@ndurell ndurell requested a review from a team as a code owner January 4, 2024 18:43
@ndurell ndurell requested a review from ajaysubra January 4, 2024 18:43
Comment on lines +99 to +101
identifiers = nil
self.value = value
time = environment.analytics.date()
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be self.identifiers and self.time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I'm totally following but you don't have to use self if it's clear what you are setting. I think I have a swiftlint rule that will remove this but I can try setting it.

@ndurell ndurell merged commit c3f3f2a into master Jan 10, 2024
7 checks passed
@ndurell ndurell deleted the noah/deprecate_event_identifiers branch January 10, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants