villebro commented on a change in pull request #16701:
URL: https://github.com/apache/superset/pull/16701#discussion_r710807844
##########
File path:
superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts
##########
@@ -25,10 +25,6 @@ describe('Dashboard edit markdown', () => {
});
it('should load AceEditor on demand', () => {
- let numScripts = 0;
- cy.get('script').then(nodes => {
- numScripts = nodes.length;
- });
Review comment:
nit: we should maybe update the test name to reflect the fact that we're
testing the edit feature.
##########
File path:
superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts
##########
@@ -26,10 +26,6 @@ describe('AdhocFilters', () => {
cy.verifySliceSuccess({ waitAlias: '@postJson' });
});
- xit('Should not load mathjs when not needed', () => {
- cy.get('script[src*="mathjs"]').should('have.length', 0);
- });
-
Review comment:
Note to other reviewers: we're fully removing `mathjs` as a dependency
right now due to bundle size and compatibility problems with Webpack 5. Related
PR removing `mathjs` from `superset-ui`:
https://github.com/apache-superset/superset-ui/pull/1362
##########
File path: superset-frontend/webpack.config.js
##########
@@ -141,17 +152,6 @@ if (!process.env.CI) {
plugins.push(new webpack.ProgressPlugin());
}
-// clean up built assets if not from dev-server
-if (!isDevServer) {
- plugins.push(
- new CleanWebpackPlugin({
- dry: false,
- // required because the build directory is outside the frontend
directory:
- dangerouslyAllowCleanPatternsOutsideProject: true,
- }),
- );
-}
-
Review comment:
Is this no longer needed? I remember this having been a problem before
causing lots of accumulated old bundles in the build directory.
##########
File path:
superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
##########
@@ -204,10 +204,25 @@ export default class AnnotationLayer extends
React.PureComponent {
return sources;
}
- isValidFormula(value, annotationType) {
+ isValidFormula(expression, annotationType) {
Review comment:
bycatch: let's either rename this to `isInvalidFormula` or flip the
return values around (this would be my preference).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
