-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
(WIP) Mtopo27/size analysis docs #15256
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Bundle ReportChanges will increase total bundle size by 86.64kB (0.37%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-client-array-pushAssets Changed:
view changes for bundle: sentry-docs-server-cjsAssets 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.
This looks great! I gave it a thorough read.
I think it will be more straightforward if we stick to our assumptions.
For example, if we say "assume the sentry-cli is installed" and then we say "apply the SENTRY_ACCESS_TOKEN" env variable, this will probably be a unnecessary step because they are already authed, but if they authed some other way, they might think the sentry-cli requires env variable auth token ONLY. So I just went and made sure if we are assuming something to not repeat the instructions and just link to other existing instructions.
I will give it another pass after this is applied so that I can see it visually in the deployment.
| ## Set your auth token | ||
|
|
||
| ```bash | ||
| export SENTRY_AUTH_TOKEN=your-token-here |
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.
Since we are already assuming they have the sentry-cli installed, I will assume they have this already set up.
Otherwise, there are many ways to auth the sentry-cli. The easiest is probably running sentry-cli login so otherwise we should link to both installation and configuration of the sentry-cli and remove this section.
| export SENTRY_AUTH_TOKEN=your-token-here |
|
|
||
| ```groovy {filename:build.gradle} | ||
| plugins { | ||
| id "io.sentry.android.gradle" version "6.0.0-alpha.4" |
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.
| id "io.sentry.android.gradle" version "6.0.0-alpha.4" | |
| id "io.sentry.android.gradle" version "6.0.0-alpha.5" |
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.
I agree that we should state an assumption for having sentry-cli installed or whatever else they need for it to work, like having environment variables set in CI.
To that point, it might make sense to split this into two sections: setup and local test, and then CI setup. It's a bit unclear why the second step talks about GitHub Actions env, and the the following step is to build the app locally.
It might flow better to do the setup and test it locally, then have a following section for setting it up for CI. This would have the benefit of getting them to first result faster in the first section.
| @@ -0,0 +1,17 @@ | |||
| ### Sentry CLI (Any platform) | |||
|
|
|||
| ```bash | |||
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.
Just FYI in case you weren't aware, you can filter the content in includes by the current platform area being viewed, that way we don't display android instructions in Apple docs and vice-versa:
| ```bash | |
| <PlatformSection supported={["apple"]}> | |
| ```bash |
and same below for android. You'd have to split the code block into two different code blocks surrounded by these. I tried doing it all in one GH suggestion for you but the triple backticks are colliding so it's not possible and it doesn't render correctly here, but if you edit my comment you'll see the complete code, including the </PlatformSection>s:
<PlatformSection supported={["apple"]}>
export SENTRY_AUTH_TOKEN=your-token-here
# iOS
sentry-cli build upload app.xcarchive.zip \
--org your-org \
--project your-project \
--build-configuration Release<PlatformSection supported={["android"]}>
export SENTRY_AUTH_TOKEN=your-token-here
# Android
sentry-cli build upload app.aab \
--org your-org \
--project your-project \
--build-configuration freeReleaseThere 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.
oh we should do something similar for build.gradle vs build.gradle.kts
| ``` | ||
| ## Build your app | ||
| ## Building your app will trigger an upload |
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.
i think we should say that building an APK or bundle will trigger the upload on CI with the configuration.
| id("io.sentry.android.gradle") version "6.0.0-alpha.5" | ||
| } | ||
|
|
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.
Let’s just remove it from the build.gradle.kts here too since we removed it from the build.gradle.
docs/product/size-analysis/index.mdx
Outdated
|
|
||
| Set up the [GitHub integration](/product/size-analysis/github-integration/) to receive size change notifications as PR comments and status checks, making size considerations a natural part of your code review process. | ||
|
|
||
| ## Supported Platforms |
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.
Shall we mention something about the hybrids (RN, Flutter) that they also work through the native uploaded bundles?
| To upload builds you'll need: | ||
|
|
||
| 1. A Sentry account with access to any [core plan](https://sentry.io/pricing/) | ||
| 2. A Sentry auth token – [generate one here](https://sentry.sentry.io/settings/auth-tokens/) and set `SENTRY_AUTH_TOKEN` in your environment so the tools can pick it up automatically |
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.
This is our org, right? Shall we maybe embed a snippet where you can generate the auth token right in the docs, like here?
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.
This is a good point but I think we should just link to the docs on authenticating the CLI since using an environment variable might not be the preferred way to authenticate. (There are 4 other ways to provide the token).
|
|
||
|  | ||
|
|
||
| The Fastlane plugin automatically detects all build metadata. If needed, the metadata values can be overriden by passing parameters to `sentry_upload_build`: |
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 Fastlane plugin automatically detects all build metadata. If needed, the metadata values can be overriden by passing parameters to `sentry_upload_build`: | |
| The Fastlane plugin automatically detects all build metadata. If needed, the metadata values can be overridden by passing parameters to `sentry_upload_build`: |
Co-authored-by: Alex Krawiec <[email protected]>
| description: Preview how Size Analysis highlights Flutter build trends. | ||
| --- | ||
|
|
||
| Size Analysis Insights point out how opportunities to reduce your Flutter app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. |
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.
| Size Analysis Insights point out how opportunities to reduce your Flutter app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. | |
| Size Analysis Insights point out opportunities to reduce your Flutter app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. |
|
|
||
| **Upload Mechanisms**: [Sentry CLI](#uploading-ios-builds-with-the-sentry-cli) | [Fastlane](#uploading-ios-builds-with-fastlane) | ||
|
|
||
| ### Uploading iOS builds with the Sentry CLI |
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.
| ### Uploading iOS builds with the Sentry CLI | |
| ### Uploading iOS Builds With the Sentry CLI |
|
|
||
| <Include name="size-analysis/upload-cli-ios" /> | ||
|
|
||
| ### Uploading iOS builds with Fastlane |
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.
| ### Uploading iOS builds with Fastlane | |
| ### Uploading iOS Builds With Fastlane |
|
|
||
| <Include name="size-analysis/upload-cli-android" /> | ||
|
|
||
| ### Uploading Android builds with Gradle |
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.
| ### Uploading Android builds with Gradle | |
| ### Uploading Android Builds With Gradle |
|
|
||
| **Upload Mechanisms**: [Sentry CLI](#uploading-ios-builds-with-the-sentry-cli) | [Fastlane](#uploading-ios-builds-with-fastlane) | ||
|
|
||
| ### Uploading iOS builds with the Sentry CLI |
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.
| ### Uploading iOS builds with the Sentry CLI | |
| ### Uploading iOS Builds With the Sentry CLI |
|
|
||
| <Include name="size-analysis/upload-cli-ios" /> | ||
|
|
||
| ### Uploading iOS builds with Fastlane |
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.
| ### Uploading iOS builds with Fastlane | |
| ### Uploading iOS Builds With Fastlane |
|
|
||
| <Include name="size-analysis/upload-cli-android" /> | ||
|
|
||
| ### Uploading Android builds with Gradle |
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.
| ### Uploading Android builds with Gradle | |
| ### Uploading Android Builds With Gradle |
| description: See how Size Analysis surfaces trends for React Native builds. | ||
| --- | ||
|
|
||
| Size Analysis Insights point out how opportunities to reduce your React Native app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. |
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.
| Size Analysis Insights point out how opportunities to reduce your React Native app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. | |
| Size Analysis Insights point out opportunities to reduce your React Native app's size. They spot patterns like duplicate files, oversized media, or unneeded assets, and list exactly what to fix along with the estimated size savings. |
| @@ -0,0 +1,97 @@ | |||
| --- | |||
| title: Integrating into CI | |||
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.
| title: Integrating into CI | |
| title: Integrating Into CI |
|
|
||
| ## Troubleshooting | ||
|
|
||
| ### Not seeing status checks |
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.
| ### Not seeing status checks | |
| ### Not Seeing Status Checks |
| - Your builds that you want to compare have the same build configuration (for example, both are `Release` builds) | ||
| - Your builds contain all the [required metadata](/product/size-analysis/#upload-metadata) | ||
|
|
||
| ### Missing base build |
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.
| ### Missing base build | |
| ### Missing Base Build |
| 1. Merge a PR or push to your main branch to trigger a build | ||
| 2. Future PRs will be able to compare against this base build | ||
|
|
||
| ### Unexpected size changes show in the comparison |
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.
| ### Unexpected size changes show in the comparison | |
| ### Unexpected Size Changes Show in the Comparison |
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.
@mtopo27 Thanks, this was a big lift! Added several small suggestions, but this looks good to go. Once it passes the typo/404 checks it looks good to merge.
DESCRIBE YOUR PR
adding docs for Size Analysis funcitonality
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:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES