ref(insights): rename resource module to asset module #70951
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Frontend
Bundle ReportChanges will increase total bundle size by 6.09kB ⬆️
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #70951 +/- ##
===========================================
+ Coverage 64.59% 77.90% +13.31%
===========================================
Files 6520 6548 +28
Lines 291600 291902 +302
Branches 50418 50424 +6
===========================================
+ Hits 188350 227420 +39070
+ Misses 96767 58232 -38535
+ Partials 6483 6250 -233
|
DominikB2014
commented
May 29, 2024
| export const BASE_URL = 'browser/resources'; | ||
| export const DATA_TYPE = t('Asset'); // Name of the data shown (singular) |
There was a problem hiding this comment.
Had trouble finding a decent name for this, but its a variable that represents the singular version of the type of data on the page. It maps to the span, i.e were showing Asset spans in this module
DominikB2014
commented
May 29, 2024
static/app/routes.tsx
Outdated
Comment on lines
1511
to
1526
| <Route path={`${PERFORMANCE_MODULE_BASE_URLS[ModuleName.RESOURCE]}/`}> | ||
| <IndexRoute | ||
| component={make( | ||
| () => import('sentry/views/performance/browser/resources/index') | ||
| )} | ||
| /> | ||
| <Route | ||
| path="spans/span/:groupId/" | ||
| component={make( | ||
| () => | ||
| import( | ||
| 'sentry/views/performance/browser/resources/resourceSummaryPage/index' | ||
| ) | ||
| )} | ||
| /> | ||
| </Route> |
There was a problem hiding this comment.
/browser/assets and /browser/resources will just route to the same component so we don't have to rely on the organization to setup routes
DominikB2014
commented
May 29, 2024
Comment on lines
+9
to
+14
| export const PERFORMANCE_MODULE_TITLE = t('Resources'); | ||
| export const PERFORMANCE_BASE_URL = 'browser/resources'; | ||
| export const PERFORMANCE_DATA_TYPE = t('Resource'); | ||
| export const PERFORMANCE_MODULE_DESCRIPTION = t( | ||
| 'Find large and slow-to-load resources used by your application and understand their impact on page performance.' | ||
| ); |
There was a problem hiding this comment.
Prefixed with Performance so that once we move to insights, we just delete these and it all makes sense still. If we prefixed with insights, we would either have to live with it, or refactor this variable.
gggritso
reviewed
May 29, 2024
Co-authored-by: George Gritsouk <[email protected]>
gggritso
approved these changes
May 29, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Renaming the
resourcesmodule toassetsresonates better with FE devs. This does all the renaming in sentry itself, and there's another PR up for docs getsentry/sentry-docs#10047