-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ui: finish short-term enhancements to cluster overview #20669
ui: finish short-term enhancements to cluster overview #20669
Conversation
Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed. pkg/ui/src/views/cluster/containers/clusterOverview/index.tsx, line 17 at r1 (raw file):
does Comments from Reviewable |
Well look at that, your nit made me learn something. Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions. pkg/ui/src/views/cluster/containers/clusterOverview/index.tsx, line 17 at r1 (raw file): Previously, vilterp (Pete Vilter) wrote…
TBH I just cargo-culted that 😳. Tried it out. Builds fine (Webpack gets it), but tslint (and maybe tsc) doesn't get it: ... UPDATE: after reading through microsoft/TypeScript#2709 I've found a solution! I'm not exactly clear on how the Comments from Reviewable |
9b81e20
to
1cd32d6
Compare
Nice 👍 LGTM |
whoops, missed the file with the important part |
16451c9
to
26ebba7
Compare
Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. pkg/ui/src/views/cluster/containers/clusterOverview/index.tsx, line 17 at r1 (raw file): Previously, couchand (Andrew Couch) wrote…
RE: Side effect imports, yes, that is correct. Comments from Reviewable |
Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. Comments from Reviewable |
Release note: None
Release note: None
Closes cockroachdb#19512. Release note: None
Previously, require was used to get around TypeScript's checking of imported modules. But this is ugly, and fortunately we can avoid it by declaring a module for anything in the assets dir. Release note: None
26ebba7
to
ede0950
Compare
This fixes everything from #19512, leaving #20540 and #19945 for future work.