-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Adds traces overview with mock data #22628
Merged
Merged
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d588f67
Adds traces overview with mock data
jasonrhodes b48ea09
Updates service overview snapshots
jasonrhodes 2c64840
Updates based on review feedback
jasonrhodes fa926f6
Adds tests for ManagedTable and ImpactBar
jasonrhodes eea7dca
Refactored transaction overview to use new managed table component
jasonrhodes e62d105
Fixes tab size
jasonrhodes 893d936
Cleans up some tests and types
jasonrhodes 89c9b3e
Cleans up types and tests
jasonrhodes a805f4c
kbn bootstrap changes to x-pack yarn.lock
jasonrhodes 0600e6e
Removed jsconfig file in apm
jasonrhodes 531777d
Updates snapshot tests
jasonrhodes a1df24d
Reversed bogus yarn lock change in x-pack
jasonrhodes c7a39a3
review feedback wip
jasonrhodes 05a7188
Resolves typescript issues
jasonrhodes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
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.
Problem here: by using
^
for this package, installing another@types/*
package that depends on@types/react
using the*
semver range makes us upgrade to the latest version that our ranges allow, so installing@types/react-router-dom
auto-upgraded this package to 16.4.x, and that version breaks a lot of our types stuff since we still use React 16.3.Switching to
~
didn't help, because even upgrading to 16.3.18 breaks tons of our kibana typescript stuff. I don't think the@types/*
packages really follow semver the same way as other packages do, similarly to how linting rulesets can't really follow semver, because every single change is most likely breaking in some way.tl;dr since newer versions of
@types/react
cause lots of typescript errors in kibana, we should be explicit about the version that works with kibana and specify16.3.14
directly.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.
Makes sense! Let's just double check with @spalger or @epixa to see if they have any objections.
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.
Sounds good, I'm going to merge this now since it's not actually changing anything with the real deps, and if there are objections or considerations we can revert the package.json change later if we need to. Thanks!
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.
No objections. It seems strange that it breaks so much though.