Skip to content

Conversation

zerts
Copy link
Collaborator

@zerts zerts commented Sep 4, 2025

No description provided.

Copy link

github-actions bot commented Sep 4, 2025

📦 build.zip [updated at Sep 9, 11:30:09 AM UTC]

@zerts zerts force-pushed the task/extension-add-option-to-disable-analytics-WLT-8206 branch from abd89c8 to 0fe2a24 Compare September 5, 2025 13:30
@zerts zerts marked this pull request as ready for review September 8, 2025 14:00
@zerts zerts requested a review from everdimension September 8, 2025 14:00
Comment on lines +112 to +114
if (!explicitly) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

should this condition come first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is fair. My only logic was to place this settings check as same as possible everywhere to simplify the usage

Comment on lines +242 to +245
const preferences = await globalPreferences.getPreferences();
if (!preferences.analyticsEnabled) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

This check is repeated over and over. Do you think we can create a helper that abstracts this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Discussed and decided to keep it this way for now

@zerts zerts merged commit 6fcebb9 into main Sep 15, 2025
3 checks passed
@zerts zerts deleted the task/extension-add-option-to-disable-analytics-WLT-8206 branch September 15, 2025 11:57
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