Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[FSSDK-10758] implement vuid configuration independent of odp #497

Merged
merged 9 commits into from
Nov 19, 2024

Conversation

raju-opti
Copy link
Contributor

Summary

  • With this PR, VUID can be enabled/disabled independently of ODP

Test plan

  • added news tests and updated affected old tests

Issues

  • FSSDK-10758

Copy link
Contributor

@muzahidul-opti muzahidul-opti left a comment

Choose a reason for hiding this comment

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

Looks good to me, where do we check client_initialized event based on vuid enable status.

@raju-opti
Copy link
Contributor Author

Looks good to me, where do we check client_initialized event based on vuid enable status.

this was already implemented here:

This will drop any event if there is no valid identifier but there will be a error log. I have added a new condition to check whether the vuid is not null to avoid the log: 3512a65

@muzahidul-opti

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM - with a small fix suggestion

@Synchronized
fun configure(enableVuid: Boolean, context: Context) {
if (!enableVuid) {
removeVuid(context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
removeVuid(context)
removeVuid(context)
self.vuid = null

@raju-opti raju-opti merged commit 7bfd707 into master Nov 19, 2024
11 of 12 checks passed
@raju-opti raju-opti deleted the raju/vuid branch November 19, 2024 11:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants