Skip to content

Conversation

itssharmasandeep
Copy link
Contributor

Description

Adding error column in trace table

Testing

Local testing is done.

Checklist:

  • My changes generate no new warnings

Screenshot 2021-04-20 at 3 35 15 PM

@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #786 (05545e5) into main (fa80f9a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #786   +/-   ##
=======================================
  Coverage   85.42%   85.42%           
=======================================
  Files         792      792           
  Lines       16161    16161           
  Branches     2060     2060           
=======================================
  Hits        13805    13805           
  Misses       2325     2325           
  Partials       31       31           
Impacted Files Coverage Δ
...apis/api-detail/traces/api-trace-list.dashboard.ts 100.00% <ø> (ø)
...vice-detail/traces/service-trace-list.dashboard.ts 100.00% <ø> (ø)
...y/src/pages/explorer/explorer-dashboard-builder.ts 88.88% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa80f9a...05545e5. Read the comment docs.

@github-actions

This comment has been minimized.

JBAhire
JBAhire previously approved these changes Apr 20, 2021
Copy link
Member

@JBAhire JBAhire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Are we settling down on Errors for column name then?

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@anandtiwary
Copy link
Contributor

anandtiwary commented Apr 20, 2021

@itssharmasandeep since you added new columns, you would probably have to update the expected model json in this test

https://github.com/hypertrace/hypertrace-ui/pull/786/checks?check_run_id=2392888321

FAIL projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts
● Explorer dashboard builder › can build dashboard JSON for traces

expect(received).toEqual(expected) // deep equality

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

@arjunlalb arjunlalb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions

This comment has been minimized.

@itssharmasandeep itssharmasandeep merged commit 782a58f into main Apr 22, 2021
@itssharmasandeep itssharmasandeep deleted the trace-error-column branch April 22, 2021 05:12
@github-actions
Copy link

Unit Test Results

    4 files  ±0  250 suites  ±0   13m 42s ⏱️ - 2m 43s
897 tests ±0  897 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
903 runs  ±0  903 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 782a58f. ± Comparison against base commit fa80f9a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants