fix(metrics): Add missing 'return' statement in <WidgetDetails /> #70512
Conversation
This file contains hidden or 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
Add a missing 'return' statement in the rendering logic of the <WidgetDetails /> component defined in 'static/app/views/metrics'.
github-actions
bot
added
the
Scope: Frontend
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #70512 +/- ##
==========================================
+ Coverage 79.97% 80.00% +0.03%
==========================================
Files 6495 6495
Lines 289790 289919 +129
Branches 49928 49947 +19
==========================================
+ Hits 231753 231950 +197
+ Misses 57625 57558 -67
+ Partials 412 411 -1
|
anonrig
approved these changes
May 8, 2024
evanpurkhiser
added
the
Trigger: getsentry tests
|
/gcbrun |
edwardgou-sentry
approved these changes
May 14, 2024
There was a problem hiding this comment.
Code looks fine, but I don't think this actually changes anything because we return MetricDetails on L60 anyways, and those props are optional.
cc: @obostjancic might be more familiar with the metrics page
|
Thanks @evanpurkhiser and @edwardgou-sentry for the review! Do you think we can get this merged? |
ArthurKnaus
approved these changes
May 21, 2024
There was a problem hiding this comment.
@edwardgou-sentry is right this will not change any behaviour. Though it is cleaner.
thx for fixing @floels 🙌
We should use the proper type guards here. Will do in a follow-up.
ArthurKnaus
added a commit
that referenced
this pull request
May 21, 2024
Use type guard and remove casting in widget details. - follow-up of #70512
cmanallen
pushed a commit
that referenced
this pull request
May 21, 2024
…0512) # Goal The goal of this PR is to add a missing `return` statement in the rendering logic of the `<WidgetDetails />` component defined in `static/app/views/metrics`: https://github.com/getsentry/sentry/blob/6b530ffab46d0b50512f0f4c9df8d084c1feb831/static/app/views/metrics/widgetDetails.tsx#L54-L56 This `return` was most likely forgotten when writing this piece of code. This probably doesn't result in any bug: by incorrectly continuing the rendering function, we end up also rendering a `<MetricDetails />` component with `undefined` `mri`, `op` and `query`. The code makes more sense with the `return` statement though. # 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.
cmanallen
pushed a commit
that referenced
this pull request
May 21, 2024
Use type guard and remove casting in widget details. - follow-up of #70512
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.
Goal
The goal of this PR is to add a missing
returnstatement in the rendering logic of the<WidgetDetails />component defined instatic/app/views/metrics:sentry/static/app/views/metrics/widgetDetails.tsx
Lines 54 to 56 in 6b530ff
This
returnwas most likely forgotten when writing this piece of code. This probably doesn't result in any bug: by incorrectly continuing the rendering function, we end up also rendering a<MetricDetails />component withundefinedmri,opandquery.The code makes more sense with the
returnstatement though.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.