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

feat: add method to get device ID synchronously #490

Merged
merged 7 commits into from
Apr 4, 2024
Merged

Conversation

bgiori
Copy link
Collaborator

@bgiori bgiori commented Apr 3, 2024

Summary

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

Sources/Amplitude/Amplitude.m Outdated Show resolved Hide resolved
@bgiori
Copy link
Collaborator Author

bgiori commented Apr 3, 2024

Plan on adding a test case here too.

Copy link
Collaborator

@crleona crleona left a comment

Choose a reason for hiding this comment

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

I don't think this needs to be a separate method - initializeDeviceID has no dependencies other than dbhelper which is set up at init, so it should be safe to call early from the regular accessor if the deviceID is nil.

It looks like the late binding is just to avoid db access on the main thread, if someone needs it earlier, we don't need to defer the work.

@@ -1669,19 +1669,21 @@ - (NSString *)getAdSupportID {
}

- (NSString *)getDeviceId {
return self.deviceId;
return [self initializeDeviceId];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@crleona So something like this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, this works - we could only call initializeDeviceId on a nil check, but the overhead should be negligible and I doubt we're going to encounter any thread contention.

Copy link
Collaborator

@crleona crleona left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bgiori bgiori marked this pull request as ready for review April 4, 2024 01:47
@bgiori bgiori merged commit f5066b2 into main Apr 4, 2024
3 checks passed
@bgiori bgiori deleted the get-device-id-sync branch April 4, 2024 15:02
github-actions bot pushed a commit that referenced this pull request Apr 4, 2024
# [8.19.0](v8.18.3...v8.19.0) (2024-04-04)

### Features

* add method to get device ID synchronously ([#490](#490)) ([f5066b2](f5066b2))
Copy link

github-actions bot commented Apr 4, 2024

🎉 This PR is included in version 8.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants