You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/04 04:06:42 UTC

[GitHub] [incubator-superset] ktmud opened a new pull request #10790: refactor: merge/upgrade superset-ui packages

ktmud opened a new pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790


   ### SUMMARY
   
   Merge core `@superset-ui/*` packages into one single package. For details, see: https://github.com/apache-superset/superset-ui/pull/768
   
   This PR does two things and two things only:
   
   1. Bump dependency versions for `@superset-ui/core`, `@superset-ui/chart-controls` and all the viz plugins.
   2. Update the corresponding import paths.
   
   Any changes not related to this two should be called out. Everything should work exactly like before. 
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   
   CI + Manual verification on major visualization related pages.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689247615


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=h1) Report
   > Merging [#10790](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cda232bf1511c3078f63ebfbbe4d875012799ded?el=desc) will **decrease** coverage by `0.02%`.
   > The diff coverage is `87.50%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10790/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10790      +/-   ##
   ==========================================
   - Coverage   60.83%   60.81%   -0.03%     
   ==========================================
     Files         802      802              
     Lines       37806    37781      -25     
     Branches     3557     3557              
   ==========================================
   - Hits        22998    22975      -23     
   + Misses      14622    14620       -2     
     Partials      186      186              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `61.57% <87.50%> (-0.07%)` | :arrow_down: |
   | #python | `60.36% <ø> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `59.61% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/components/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FwcC5qc3g=) | `77.77% <ø> (ø)` | |
   | [.../src/SqlLab/components/EstimateQueryCostButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0VzdGltYXRlUXVlcnlDb3N0QnV0dG9uLmpzeA==) | `17.39% <ø> (ø)` | |
   | [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `13.33% <ø> (ø)` | |
   | [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `74.07% <ø> (ø)` | |
   | [...-frontend/src/SqlLab/components/HighlightedSql.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0hpZ2hsaWdodGVkU3FsLmpzeA==) | `90.00% <ø> (ø)` | |
   | [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <ø> (ø)` | |
   | [...et-frontend/src/SqlLab/components/QueryHistory.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5SGlzdG9yeS5qc3g=) | `83.33% <ø> (ø)` | |
   | [...set-frontend/src/SqlLab/components/QuerySearch.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5U2VhcmNoLmpzeA==) | `57.54% <ø> (ø)` | |
   | ... and [207 more](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=footer). Last update [cda232b...9127442](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter commented on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689247615


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=h1) Report
   > Merging [#10790](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cda232bf1511c3078f63ebfbbe4d875012799ded?el=desc) will **decrease** coverage by `0.52%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10790/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10790      +/-   ##
   ==========================================
   - Coverage   60.83%   60.30%   -0.53%     
   ==========================================
     Files         802      373     -429     
     Lines       37806    23822   -13984     
     Branches     3557        0    -3557     
   ==========================================
   - Hits        22998    14367    -8631     
   + Misses      14622     9455    -5167     
   + Partials      186        0     -186     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `?` | |
   | #python | `60.30% <ø> (-0.05%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.66% <0.00%> (-0.84%)` | :arrow_down: |
   | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.24% <0.00%> (-0.25%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.46% <0.00%> (-0.14%)` | :arrow_down: |
   | [...nd/src/dashboard/util/getDetailedComponentWidth.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERldGFpbGVkQ29tcG9uZW50V2lkdGguanM=) | | |
   | [...ntend/src/dashboard/util/getRevertedFilterScope.ts](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldFJldmVydGVkRmlsdGVyU2NvcGUudHM=) | | |
   | [...ntend/src/dashboard/util/activeDashboardFilters.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2FjdGl2ZURhc2hib2FyZEZpbHRlcnMuanM=) | | |
   | [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | | |
   | [...-frontend/src/dashboard/containers/FilterScope.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0ZpbHRlclNjb3BlLmpzeA==) | | |
   | [...nd/src/dashboard/components/gridComponents/Row.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL1Jvdy5qc3g=) | | |
   | ... and [210 more](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=footer). Last update [cda232b...9127442](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485299902



##########
File path: superset-frontend/cypress-base/cypress/integration/explore/visualizations/big_number.test.js
##########
@@ -43,7 +43,7 @@ describe('Visualization > Big Number with Trendline', () => {
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({
       waitAlias: '@getJson',
-      chartSelector: '.big_number',
+      chartSelector: '.superset-legacy-chart-big-number',

Review comment:
       `SupersetChart` may render before the chart content is rendered, so Cypress will find an empty `div.big_number` (the wrapper of the actual chart), breaking the test.

##########
File path: superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts
##########
@@ -30,12 +30,12 @@ describe('AdhocFilters', () => {
 
     cy.get('[data-test=adhoc_filters]').within(() => {
       cy.get('.Select__control').click();
-      cy.get('input[type=text]').type('name{enter}');
+      cy.get('input[type=text]').focus().type('name{enter}');

Review comment:
       Fixing an `input is covered by "Choose a metric..." placeholder` error.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485391060



##########
File path: superset-frontend/src/setup/setupColors.js
##########
@@ -16,19 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import airbnb from '@superset-ui/color/esm/colorSchemes/categorical/airbnb';
-import categoricalD3 from '@superset-ui/color/esm/colorSchemes/categorical/d3';
-import echarts from '@superset-ui/color/esm/colorSchemes/categorical/echarts';
-import google from '@superset-ui/color/esm/colorSchemes/categorical/google';
-import lyft from '@superset-ui/color/esm/colorSchemes/categorical/lyft';
-import preset from '@superset-ui/color/esm/colorSchemes/categorical/preset';
-import sequentialCommon from '@superset-ui/color/esm/colorSchemes/sequential/common';
-import sequentialD3 from '@superset-ui/color/esm/colorSchemes/sequential/d3';
+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';

Review comment:
       I don't feel like doing additional refactor in this already humongous PR. For this specific case, I almost feel these colors should probably be move out of the `@superset-ui/core` package and placed under `incubator-superset` instead.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud merged pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] villebro commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485392753



##########
File path: superset-frontend/src/setup/setupColors.js
##########
@@ -16,19 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import airbnb from '@superset-ui/color/esm/colorSchemes/categorical/airbnb';
-import categoricalD3 from '@superset-ui/color/esm/colorSchemes/categorical/d3';
-import echarts from '@superset-ui/color/esm/colorSchemes/categorical/echarts';
-import google from '@superset-ui/color/esm/colorSchemes/categorical/google';
-import lyft from '@superset-ui/color/esm/colorSchemes/categorical/lyft';
-import preset from '@superset-ui/color/esm/colorSchemes/categorical/preset';
-import sequentialCommon from '@superset-ui/color/esm/colorSchemes/sequential/common';
-import sequentialD3 from '@superset-ui/color/esm/colorSchemes/sequential/d3';
+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';

Review comment:
       Yeah, moving these to `incubator-superset` probably makes more sense. Also not recommending changing this now, this was mostly a question and proposal for future changes post merge.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] kristw commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485864568



##########
File path: superset-frontend/src/setup/setupColors.js
##########
@@ -16,19 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import airbnb from '@superset-ui/color/esm/colorSchemes/categorical/airbnb';
-import categoricalD3 from '@superset-ui/color/esm/colorSchemes/categorical/d3';
-import echarts from '@superset-ui/color/esm/colorSchemes/categorical/echarts';
-import google from '@superset-ui/color/esm/colorSchemes/categorical/google';
-import lyft from '@superset-ui/color/esm/colorSchemes/categorical/lyft';
-import preset from '@superset-ui/color/esm/colorSchemes/categorical/preset';
-import sequentialCommon from '@superset-ui/color/esm/colorSchemes/sequential/common';
-import sequentialD3 from '@superset-ui/color/esm/colorSchemes/sequential/d3';
+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';

Review comment:
       we could also add the export from index and get rid of deep import later.
   Agree not doing it in this humongous PR




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485280100



##########
File path: superset-frontend/package.json
##########
@@ -22,6 +22,8 @@
     "prettier-check": "prettier --check '{src,stylesheets}/**/*.{css,less,sass,scss}'",
     "lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx,.ts,tsx . && npm run clean-css && npm run type",
     "clean-css": "prettier --write '{src,stylesheets}/**/*.{css,less,sass,scss}'",
+    "format": "prettier --write './{src,spec,stylesheets,cypress-base}/**/*{.js,.jsx,.ts,.tsx,.css,.less,.scss,.sass}'",
+    "prettier": "npm run format",

Review comment:
       Adding a shortcut command to format files with Prettier, to be consistent with `superset-ui`

##########
File path: superset-frontend/package.json
##########
@@ -149,6 +140,7 @@
     "react-hot-loader": "^4.12.20",
     "react-json-tree": "^0.11.2",
     "react-jsonschema-form": "^1.2.0",
+    "react-loadable": "^5.5.0",

Review comment:
       Previously a dependency of `@superset-ui/chart`, adding it here because it was [moved](https://github.com/apache-superset/superset-ui/pull/774) to `peerDependencies` of `@superset-ui/core`.

##########
File path: superset-frontend/package.json
##########
@@ -118,6 +108,7 @@
     "dnd-core": "^2.6.0",
     "dom-to-image": "^2.6.0",
     "geolib": "^2.0.24",
+    "global-box": "^1.2.0",

Review comment:
       A new `peerDependency` from the updated `encodable`, which is a dependency of `preset-chart-xy`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485391060



##########
File path: superset-frontend/src/setup/setupColors.js
##########
@@ -16,19 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import airbnb from '@superset-ui/color/esm/colorSchemes/categorical/airbnb';
-import categoricalD3 from '@superset-ui/color/esm/colorSchemes/categorical/d3';
-import echarts from '@superset-ui/color/esm/colorSchemes/categorical/echarts';
-import google from '@superset-ui/color/esm/colorSchemes/categorical/google';
-import lyft from '@superset-ui/color/esm/colorSchemes/categorical/lyft';
-import preset from '@superset-ui/color/esm/colorSchemes/categorical/preset';
-import sequentialCommon from '@superset-ui/color/esm/colorSchemes/sequential/common';
-import sequentialD3 from '@superset-ui/color/esm/colorSchemes/sequential/d3';
+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';

Review comment:
       I don't feel like doing additional refactor in this already humongous PR. For this specific case, I almost feel these colors should probably be move out of the `@superset-ui/core` package and placed under `incubator-superset` instead so that adding a new color wouldn't require publishing the package and custom Superset installations can also opt out colors more thoroughly. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689778461


   > > even though an unrelated python test was failing for no reason.
   > 
   > CI is green now. Is it just flaky test?
   
   I think so


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] villebro commented on a change in pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#discussion_r485358696



##########
File path: superset-frontend/src/setup/setupColors.js
##########
@@ -16,19 +16,19 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import airbnb from '@superset-ui/color/esm/colorSchemes/categorical/airbnb';
-import categoricalD3 from '@superset-ui/color/esm/colorSchemes/categorical/d3';
-import echarts from '@superset-ui/color/esm/colorSchemes/categorical/echarts';
-import google from '@superset-ui/color/esm/colorSchemes/categorical/google';
-import lyft from '@superset-ui/color/esm/colorSchemes/categorical/lyft';
-import preset from '@superset-ui/color/esm/colorSchemes/categorical/preset';
-import sequentialCommon from '@superset-ui/color/esm/colorSchemes/sequential/common';
-import sequentialD3 from '@superset-ui/color/esm/colorSchemes/sequential/d3';
+import airbnb from '@superset-ui/core/esm/color/colorSchemes/categorical/airbnb';
+import categoricalD3 from '@superset-ui/core/esm/color/colorSchemes/categorical/d3';
+import echarts from '@superset-ui/core/esm/color/colorSchemes/categorical/echarts';
+import google from '@superset-ui/core/esm/color/colorSchemes/categorical/google';
+import lyft from '@superset-ui/core/esm/color/colorSchemes/categorical/lyft';
+import preset from '@superset-ui/core/esm/color/colorSchemes/categorical/preset';
+import sequentialCommon from '@superset-ui/core/esm/color/colorSchemes/sequential/common';
+import sequentialD3 from '@superset-ui/core/esm/color/colorSchemes/sequential/d3';

Review comment:
       Not that this behavior is being changed, but is there some reason these have to be pulled in from `/esm/` and aren't exported like the other components in `superset-ui/core`? It feels like these should be exported as `categoricalColorSchemes.*` and `sequentialColorSchemes.*` or similar.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689247615


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=h1) Report
   > Merging [#10790](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/50672bb11baa614ff64a376bd09a9c74d1830666?el=desc) will **decrease** coverage by `0.02%`.
   > The diff coverage is `87.50%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10790/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10790      +/-   ##
   ==========================================
   - Coverage   61.30%   61.27%   -0.03%     
   ==========================================
     Files         802      802              
     Lines       37870    37845      -25     
     Branches     3561     3561              
   ==========================================
   - Hits        23215    23190      -25     
     Misses      14469    14469              
     Partials      186      186              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `61.58% <87.50%> (-0.07%)` | :arrow_down: |
   | #python | `61.09% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `59.61% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/components/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FwcC5qc3g=) | `77.77% <ø> (ø)` | |
   | [.../src/SqlLab/components/EstimateQueryCostButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0VzdGltYXRlUXVlcnlDb3N0QnV0dG9uLmpzeA==) | `17.39% <ø> (ø)` | |
   | [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `13.33% <ø> (ø)` | |
   | [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `74.07% <ø> (ø)` | |
   | [...-frontend/src/SqlLab/components/HighlightedSql.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0hpZ2hsaWdodGVkU3FsLmpzeA==) | `90.00% <ø> (ø)` | |
   | [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <ø> (ø)` | |
   | [...et-frontend/src/SqlLab/components/QueryHistory.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5SGlzdG9yeS5qc3g=) | `83.33% <ø> (ø)` | |
   | [...set-frontend/src/SqlLab/components/QuerySearch.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5U2VhcmNoLmpzeA==) | `57.54% <ø> (ø)` | |
   | ... and [205 more](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=footer). Last update [50672bb...a3ce1e4](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689765198


   @kristw @evans @villebro This is ready for review, even though an unrelated python test was failing for no reason.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10790: refactor: merge/upgrade superset-ui packages

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10790:
URL: https://github.com/apache/incubator-superset/pull/10790#issuecomment-689247615


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=h1) Report
   > Merging [#10790](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cda232bf1511c3078f63ebfbbe4d875012799ded?el=desc) will **decrease** coverage by `0.03%`.
   > The diff coverage is `87.50%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10790/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10790      +/-   ##
   ==========================================
   - Coverage   60.83%   60.80%   -0.04%     
   ==========================================
     Files         802      802              
     Lines       37806    37792      -14     
     Branches     3557     3557              
   ==========================================
   - Hits        22998    22978      -20     
   - Misses      14622    14628       +6     
     Partials      186      186              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `61.57% <87.50%> (-0.07%)` | :arrow_down: |
   | #python | `60.34% <ø> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `59.61% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/components/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FwcC5qc3g=) | `77.77% <ø> (ø)` | |
   | [.../src/SqlLab/components/EstimateQueryCostButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0VzdGltYXRlUXVlcnlDb3N0QnV0dG9uLmpzeA==) | `17.39% <ø> (ø)` | |
   | [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `13.33% <ø> (ø)` | |
   | [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `74.07% <ø> (ø)` | |
   | [...-frontend/src/SqlLab/components/HighlightedSql.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0hpZ2hsaWdodGVkU3FsLmpzeA==) | `90.00% <ø> (ø)` | |
   | [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <ø> (ø)` | |
   | [...et-frontend/src/SqlLab/components/QueryHistory.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5SGlzdG9yeS5qc3g=) | `83.33% <ø> (ø)` | |
   | [...set-frontend/src/SqlLab/components/QuerySearch.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5U2VhcmNoLmpzeA==) | `57.54% <ø> (ø)` | |
   | ... and [212 more](https://codecov.io/gh/apache/incubator-superset/pull/10790/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=footer). Last update [cda232b...9127442](https://codecov.io/gh/apache/incubator-superset/pull/10790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org