-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
[build] Bump superset-ui packages and update build #9241
Conversation
@etr2460 Build error related to jestjs/jest#8660 |
15748d7
to
d56d04d
Compare
Yup, just saw that. Hopefully CI passes now |
9fbbf85
to
5007640
Compare
Codecov Report
@@ Coverage Diff @@
## master #9241 +/- ##
=======================================
Coverage 58.93% 58.93%
=======================================
Files 373 373
Lines 12014 12014
Branches 2945 2945
=======================================
Hits 7080 7080
Misses 4755 4755
Partials 179 179 Continue to review full report at Codecov.
|
['@babel/plugin-transform-runtime', { corejs: 3 }], | ||
], | ||
env: { | ||
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.
after all the pain of figuring this out, should we add comments for why the different test config is needed?
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.
comment added
@@ -19,6 +19,7 @@ | |||
import React from 'react'; | |||
import { shallow } from 'enzyme'; | |||
import { Overlay, Tooltip } from 'react-bootstrap'; | |||
import { promiseTimeout } from '@superset-ui/core'; |
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.
Probably not needed if you are not using it.
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.
ugh, yup. idk why a linter didn't catch this :(
superset-frontend/jest.config.js
Outdated
@@ -17,6 +17,7 @@ | |||
* under the License. | |||
*/ | |||
module.exports = { | |||
timers: 'real', |
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.
not needed. default is real
. I have tried removing and it works.
@@ -18,7 +18,7 @@ | |||
*/ | |||
import React from 'react'; | |||
import { shallow } from 'enzyme'; | |||
import sinon from 'sinon'; | |||
import { promiseTimeout } from '@superset-ui/core'; |
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.
Could use async
pattern similar to other ones for consistency.
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.
fair enough
5007640
to
6f3bb33
Compare
CATEGORY
Choose one
SUMMARY
Recently there's been some build config changes in the superset-ui repos, this pulls the new packages into Superset and updates the build to work appropriately
TEST PLAN
View a dashboard and see that charts still work as expected
ensure
npm run prod
andnpm run build
still workADDITIONAL INFORMATION
REVIEWERS
to: @kristw @graceguo-supercat @rusackas @nytai