web: remove integrations banners and corresponding pings #38715
Conversation
|
@vdavid, |
|
@taras-yemets The four PRs related to browser and IDE CTAs are: I think all four of these can be manually reverted. Note: There is a clear process for adding pings but none for removing them. As not sending data is not really a privacy concern, I'd say an RFC is not needed here. As long as all the tests pass, we're safe to remove the pings. |
|
@vdavid, thanks a lot for the insight! Will check these PRs step by step. |
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 268db33...3addad4.
|
taras-yemets
changed the title
There was a problem hiding this comment.
The changes look good to me, they should be fine if CI tests pass!
I keep on forgetting how difficult experiments are mentally: if they succeed, your code stays, if they fail, your code is removed. But I’m happy that we’re doing it the right way and remove the commit once the experiment failed! Also, it’s nice to see such a big red commit. :)
|
Thanks a lot for your help, @vdavid! |
| const [hasDismissedBrowserExtensionAlert, setHasDismissedBrowserExtensionAlert] = useTemporarySetting( | ||
| 'cta.browserExtensionAlertDismissed', | ||
| false | ||
| ) | ||
| const [hasDismissedIDEExtensionAlert, setHasDismissedIDEExtensionAlert] = useTemporarySetting( | ||
| 'cta.ideExtensionAlertDismissed', | ||
| false | ||
| ) |
There was a problem hiding this comment.
Can we remove these temporary settings keys from the schema if they're not used anymore?
| const [lastVSCodeDetection] = useTemporarySetting('integrations.vscode.lastDetectionTimestamp', 0) | ||
| const [lastJetBrainsDetection] = useTemporarySetting('integrations.jetbrains.lastDetectionTimestamp', 0) |
There was a problem hiding this comment.
Same comment here, should we remove these keys?
There was a problem hiding this comment.
Thank you for your review, @limitedmage!
Given that we removed all IDE banners, I wonder if we still need <IdeExtensionTracker /> component?
What do you think @vdavid, @philipp-spiess?
There was a problem hiding this comment.
Yes, as far as I know, only the CTAs used the IDE extension detection, so that code can go as well.
Closes https://github.com/sourcegraph/sourcegraph/issues/38625
Removes browser and IDE extensions banners from Sourcegraph website and corresponding pings.
Removed banners (screenshots in this comment):
Reverts changes introduced by the following PRs:
Test plan
sg start web-standaloneApp preview:
Check out the client app preview documentation to learn more.