-
Notifications
You must be signed in to change notification settings - Fork 513
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
Preparing release v1.13.0 #699
Preparing release v1.13.0 #699
Conversation
Signed-off-by: albertteoh <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #699 +/- ##
==========================================
- Coverage 94.42% 94.39% -0.04%
==========================================
Files 230 230
Lines 5959 5959
Branches 1448 1448
==========================================
- Hits 5627 5625 -2
- Misses 298 300 +2
Partials 34 34
Continue to review full report at Codecov.
|
I would like to get #702 in before this release. |
Signed-off-by: albertteoh <[email protected]>
@@ -42,7 +42,7 @@ describe('<SpanBarRow>', () => { | |||
serviceName: 'rpc-service-name', | |||
}, | |||
showErrorIcon: false, | |||
getViewedBounds: () => ({ start: 0, end: 1 }), | |||
getViewedBounds: () => ({ start: 10, end: 1 }), |
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.
what does it mean to start from 10 through to 1?
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 motivation for this change is to add test coverage for the following code block, which was blocking this PR from passing builds:
jaeger-ui/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanBarRow.tsx
Lines 118 to 119 in df17e8d
longLabel = `${labelDetail} | ${label}`; | |
hintSide = 'left'; |
I don't know how this code coverage issue appeared given my only changes were to the changelog, but in any case, thought I'd try to fix it as it seems to be the best option for me to unblock the release (please advise if I'm wrong).
The above code block uses the start and end view bounds to determine where the hint label should appear when mousing over the span timeline. So if there's more whitespace to the left of the span timeline than the right, show the label to the left, otherwise, show it to the right.
Realistically, these bounds seem to range between 0 and 1 and so 10 is a little extreme to be honest, but it does the job to get the line coverage for the first if
block, and it looks like the else
case is covered by other tests. Please let me know if you think this number should be more realistic or if there's a better way to fix the code coverage failure.
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.
seem to range between 0 and 1 and so 10 is a little extreme to be honest
And out of range, most importantly. I am not familiar with the test, but having 10 here is just going to confuse someone else at some time in the future.
I would remove this change. We can ignore the code cov diff for the purpose of this PR, since the diff is coming from a flaky test.
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.
Agree and reverted. The "Squash and merge" button is greyed out for me due to the codecov failure; how do we ignore it and proceed with merging this PR?
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.
booked a ticket for flaky coverage #704
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 merged it - probably requires admin rights on the repo.
This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]>
693d653
to
209fd51
Compare
Thanks @yurishkuro! |
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
* Preparing release v1.13.0 Signed-off-by: albertteoh <[email protected]> * Increase test coverage on SpanBarRow.tsx Signed-off-by: albertteoh <[email protected]> * Revert "Increase test coverage on SpanBarRow.tsx" This reverts commit 8a9592e. Signed-off-by: albertteoh <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
Signed-off-by: albertteoh [email protected]
Which problem is this PR solving?
Short description of the changes