-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(insights): Update docs for upcoming mobile screens GA #12366
chore(insights): Update docs for upcoming mobile screens GA #12366
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will increase total bundle size by 651 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
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.
thanks for updating - gave this a quick first pass and looks good already 👍
Co-authored-by: Karl Heinz Struggl <[email protected]>
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.
Thanks for this big update! Added a few suggestions, most importantly addressing a broken link. Besides that, looks good to me 🤠
Co-authored-by: Alex Krawiec <[email protected]>
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.
Left a couple minor nits. Independently, I had lost track in the shuffle that the plan was to see Mobile Screens to completion, vs., adding docs for Screen Rendering and calling it GA.
The web-vitals-like landing page looks lovely and I think is great work. We should just do a tiny bit more footwork before we go GA (I'll bring to chat, I don't see any major blockers).
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.
Thanks, @markushi. LGTM, with two minor suggestions.
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.
left small suggestions, otherwise lgtm 🚢
This is mostly a restructuring of existing content, updating some wordings and screenshots to the the latest changes for the upcoming "Mobile Screens" insights module.
Fixes getsentry/sentry#81566
Fixes getsentry/sentry#81567
Fixes getsentry/sentry#84163
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: