-
Notifications
You must be signed in to change notification settings - Fork 412
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
chore: Linkedin partner tracking #3879
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added one minor specific comment based on the implementation but I'm also keen to make sure that we've verified this doesn't cause any unnecessary outbound requests when the option is disabled, e.g. for self hosted installations. I can't see any reference to it in the test section of the PR description.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3879 +/- ##
=======================================
Coverage 95.86% 95.87%
=======================================
Files 1130 1131 +1
Lines 35795 35854 +59
=======================================
+ Hits 34315 34374 +59
Misses 1480 1480 ☔ View full report in Codecov by Sentry. |
Yeah, it wouldn't. The script only gets added to the page if the env var is set https://github.com/Flagsmith/flagsmith/pull/3879/files#diff-d2af3ec690c6d6d6c4691ed1d668ec2cf39c84b774cef27c7346e5efd555d321R106 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code LGTM but I left a comment regarding self-hosted case.
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingdocs/
if required so people know about the feature!Changes
Adds linkedin partner tracking when LINKEDIN_PARTNER_TRACKING=true
How did you test this code?