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/10/07 21:52:05 UTC
[GitHub] [incubator-superset] john-bodley opened a new pull request #11194: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
john-bodley opened a new pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194
### SUMMARY
This reverts commit 4fd993c4e07f8bd1220d105fbaa18733daf5aebb.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue: Fixes #11192
- [ ] 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-io edited a comment on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705216449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=h1) Report
> Merging [#11194](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9785667a0dc970fd7b7033f172c2760ee0c22a6e?el=desc) will **decrease** coverage by `0.16%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11194 +/- ##
==========================================
- Coverage 65.43% 65.26% -0.17%
==========================================
Files 829 828 -1
Lines 39200 39162 -38
Branches 3693 3694 +1
==========================================
- Hits 25649 25561 -88
- Misses 13444 13490 +46
- Partials 107 111 +4
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `55.09% <ø> (-0.78%)` | :arrow_down: |
| #javascript | `62.30% <ø> (-0.03%)` | :arrow_down: |
| #python | `60.60% <ø> (ø)` | |
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/11194?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...c/explore/components/controls/withVerification.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy93aXRoVmVyaWZpY2F0aW9uLmpzeA==) | | |
| [...re/components/controls/TimeSeriesColumnControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC5qc3g=) | | |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | | |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | | |
| [...perset-frontend/src/addSlice/AddSliceContainer.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2FkZFNsaWNlL0FkZFNsaWNlQ29udGFpbmVyLnRzeA==) | | |
| [...nd/src/dashboard/components/gridComponents/Row.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL1Jvdy5qc3g=) | | |
| [...d/src/dashboard/util/updateComponentParentsList.js](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3VwZGF0ZUNvbXBvbmVudFBhcmVudHNMaXN0Lmpz) | | |
| [...nd/src/messageToasts/components/ToastPresenter.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvY29tcG9uZW50cy9Ub2FzdFByZXNlbnRlci50c3g=) | | |
| [...board/components/gridComponents/new/NewDivider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL25ldy9OZXdEaXZpZGVyLmpzeA==) | | |
| [...plore/components/controls/FixedOrMetricControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaXhlZE9yTWV0cmljQ29udHJvbC5qc3g=) | | |
| ... and [856 more](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?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/11194?src=pr&el=footer). Last update [9785667...8d08be6](https://codecov.io/gh/apache/incubator-superset/pull/11194?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] etr2460 commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
etr2460 commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705707294
Personally, I think we should err on the side of keeping `master` as stable as possible, especially since many people do depend on running and developing from master. When master is broken, it not only can break deployments of Superset, but also makes developing significantly harder.
To me, reverts and re-PRs seem pretty trivial in terms of actual work required. Is there really significantly more overhead with adding a fix to a previous branch vs. fixing forward? Especially when these are clean reverts without any merge conflicts.
As for the revert guidelines, I think i've always been more aggressively pushing for reverts, perhaps even more so than the guidelines allow. But in this case, I think the guidelines were adhered to. 24hr of broken master could affect up to 20 developers and even more end users of Superset, so that seems like far too long to live in a broken state. I'd go so far as to say that if the initial committer isn't immediately available in slack, then we should move forward with reverting. Personally, if I introduced a regression in master and couldn't immediately fix it, I would much rather have it be reverted for me to come back to later than for it to live in master and slow others down
----------------------------------------------------------------
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-io edited a comment on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705216449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=h1) Report
> Merging [#11194](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9785667a0dc970fd7b7033f172c2760ee0c22a6e?el=desc) will **decrease** coverage by `4.19%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11194 +/- ##
==========================================
- Coverage 65.43% 61.23% -4.20%
==========================================
Files 829 828 -1
Lines 39200 39159 -41
Branches 3693 3694 +1
==========================================
- Hits 25649 23980 -1669
- Misses 13444 14998 +1554
- Partials 107 181 +74
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.30% <ø> (-0.03%)` | :arrow_down: |
| #python | `60.60% <ø> (ø)` | |
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/11194?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | | |
| [...ws/CRUD/data/savedquery/SavedQueryPreviewModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9zYXZlZHF1ZXJ5L1NhdmVkUXVlcnlQcmV2aWV3TW9kYWwudHN4) | | |
| [superset-frontend/src/profile/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvaW5kZXgudHN4) | | |
| [...ashboard/components/gridComponents/new/NewTabs.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL25ldy9OZXdUYWJzLmpzeA==) | | |
| [...tend/src/explore/components/DisplayQueryButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EaXNwbGF5UXVlcnlCdXR0b24uanN4) | | |
| [superset-frontend/src/components/Label/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGFiZWwvaW5kZXgudHN4) | | |
| [...nd/src/dashboard/containers/DashboardComponent.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZENvbXBvbmVudC5qc3g=) | | |
| [.../src/dashboard/components/menu/PopoverDropdown.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL21lbnUvUG9wb3ZlckRyb3Bkb3duLmpzeA==) | | |
| [...d/util/logging/getLoadStatsPerTopLevelComponent.js](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2xvZ2dpbmcvZ2V0TG9hZFN0YXRzUGVyVG9wTGV2ZWxDb21wb25lbnQuanM=) | | |
| [.../src/explore/components/controls/BoundsControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Cb3VuZHNDb250cm9sLmpzeA==) | | |
| ... and [856 more](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?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/11194?src=pr&el=footer). Last update [9785667...8d08be6](https://codecov.io/gh/apache/incubator-superset/pull/11194?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] rusackas commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705700602
Just to chime in (since I approved both the PR and the revert), I appreciate the need for a stable `master` when people are deploying it regularly and reliant on a certain standard of stability. That said, I feel we should continue working to increase the cadence of Apache releases as stable waypoints, so that `master` can be a _little_ more forgiving in this area. I agree that 24h to provide a patch seems like a reasonable window for the author (or anyone) to apply a patch, with the (documented) addition of exceptions such as newly introduced security vulnerabilities.
----------------------------------------------------------------
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] etr2460 commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
etr2460 commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705707294
Personally, I think we should err on the side of keeping `master` as stable as possible, especially since many people do depend on running and developing from master. When master is broken, it not only can break deployments of Superset, but also makes developing significantly harder.
To me, reverts and re-PRs seem pretty trivial in terms of actual work required. Is there really significantly more overhead with adding a fix to a previous branch vs. fixing forward? Especially when these are clean reverts without any merge conflicts.
As for the revert guidelines, I think i've always been more aggressively pushing for reverts, perhaps even more so than the guidelines allow. But in this case, I think the guidelines were adhered to. 24hr of broken master could affect up to 20 developers and even more end users of Superset, so that seems like far too long to live in a broken state. I'd go so far as to say that if the initial committer isn't immediately available in slack, then we should move forward with reverting. Personally, if I introduced a regression in master and couldn't immediately fix it, I would much rather have it be reverted for me to come back to later than for it to live in master and slow others down
----------------------------------------------------------------
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-io commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705216449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=h1) Report
> Merging [#11194](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9785667a0dc970fd7b7033f172c2760ee0c22a6e?el=desc) will **decrease** coverage by `4.59%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11194 +/- ##
==========================================
- Coverage 65.43% 60.83% -4.60%
==========================================
Files 829 828 -1
Lines 39200 39148 -52
Branches 3693 3694 +1
==========================================
- Hits 25649 23815 -1834
- Misses 13444 15152 +1708
- Partials 107 181 +74
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.30% <ø> (-0.03%)` | :arrow_down: |
| #python | `59.96% <ø> (-0.65%)` | :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/11194?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `81.38% <0.00%> (-7.98%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `94.77% <0.00%> (-5.23%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| ... and [881 more](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?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/11194?src=pr&el=footer). Last update [9785667...8d08be6](https://codecov.io/gh/apache/incubator-superset/pull/11194?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] rusackas commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705700602
Just to chime in (since I approved both the PR and the revert), I appreciate the need for a stable `master` when people are deploying it regularly and reliant on a certain standard of stability. That said, I feel we should continue working to increase the cadence of Apache releases as stable waypoints, so that `master` can be a _little_ more forgiving in this area. I agree that 24h to provide a patch seems like a reasonable window for the author (or anyone) to apply a patch, with the (documented) addition of exceptions such as newly introduced security vulnerabilities.
----------------------------------------------------------------
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] mistercrunch commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705672458
> Availability of the PR author: If the original PR author or the engineer who merged the code is highly available and can provide a fix in a reasonable timeframe, this would counter-indicate reverting.
That's the one item here, can we leave a minimum of ~24H after identifying the issue and reaching out to the author/merger combo on Slack?
----------------------------------------------------------------
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] etr2460 merged pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
etr2460 merged pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194
----------------------------------------------------------------
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] john-bodley edited a comment on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705266764
@mistercrunch I per the guidelines I think reverting made sense:
> Availability of the PR author: If the original PR author or the engineer who merged the code is highly available and can provide a fix in a reasonable timeframe, this would counter-indicate reverting.
Unclear. @kgabryje is a relatively new contributor.
> Severity of the issue: How severe is the problem on master? Is it keeping the project from moving forward? Is there user impact? What percentage of users will experience a problem?
SQL Lab is unusable for result sets with a moderate number of columns.
> Size of the change being reverted: Reverting a single small PR is a much lower-risk proposition than reverting a massive, multi-PR change.
The PR was self contained and reverted without conflict, thus the risk proposition seemed quite low.
> Age of the change being reverted: Reverting a recently-merged PR will be more acceptable than reverting an older PR. A bug discovered in an older PR is unlikely to be causing widespread serious issues.
The PR is relatively new and was merged 5 days ago.
> Risk inherent in reverting: Will the reversion break critical functionality? Is the medicine more dangerous than the disease? Difficulty of crafting a fix: In the case of issues with a clear solution, it may be preferable to implement and merge a fix rather than a revert.
The risk is low especially as this was a new feature and the previous state was deemed stable. It is unclear how trivial it is to craft a fix.
----------------------------------------------------------------
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] john-bodley commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705266764
@mistercrunch I per the guidelines I think reverting made sense:
> Availability of the PR author: If the original PR author or the engineer who merged the code is highly available and can provide a fix in a reasonable timeframe, this would counter-indicate reverting.
Unclear. @kgabryje is a relatively new contributor.
> Severity of the issue: How severe is the problem on master? Is it keeping the project from moving forward? Is there user impact? What percentage of users will experience a problem?
SQL Lab is unusable for result sets with a moderate number of columns.
> Size of the change being reverted: Reverting a single small PR is a much lower-risk proposition than reverting a massive, multi-PR change.
PR was relatively small and reverted without conflict.
> Age of the change being reverted: Reverting a recently-merged PR will be more acceptable than reverting an older PR. A bug discovered in an older PR is unlikely to be causing widespread serious issues.
The PR is relatively new and was merged 5 days ago.
> Risk inherent in reverting: Will the reversion break critical functionality? Is the medicine more dangerous than the disease?
Difficulty of crafting a fix: In the case of issues with a clear solution, it may be preferable to implement and merge a fix rather than a revert.
The risk is low especially as this was a new feature and the previous state was deemed stable.
----------------------------------------------------------------
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-io edited a comment on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705216449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=h1) Report
> Merging [#11194](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9785667a0dc970fd7b7033f172c2760ee0c22a6e?el=desc) will **decrease** coverage by `4.35%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11194?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11194 +/- ##
==========================================
- Coverage 65.43% 61.07% -4.36%
==========================================
Files 829 828 -1
Lines 39200 39159 -41
Branches 3693 3694 +1
==========================================
- Hits 25649 23915 -1734
- Misses 13444 15063 +1619
- Partials 107 181 +74
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.30% <ø> (-0.03%)` | :arrow_down: |
| #python | `60.34% <ø> (-0.27%)` | :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/11194?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.56% <0.00%> (-0.56%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.70% <0.00%> (-0.14%)` | :arrow_down: |
| [.../src/dashboard/components/FilterIndicatorGroup.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvckdyb3VwLmpzeA==) | | |
| [...et-frontend/src/dashboard/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NhdmVNb2RhbC5qc3g=) | | |
| [...erset-frontend/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | | |
| [...shboard/components/filterscope/FilterFieldItem.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2ZpbHRlcnNjb3BlL0ZpbHRlckZpZWxkSXRlbS5qc3g=) | | |
| ... and [862 more](https://codecov.io/gh/apache/incubator-superset/pull/11194/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11194?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/11194?src=pr&el=footer). Last update [9785667...8d08be6](https://codecov.io/gh/apache/incubator-superset/pull/11194?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] mistercrunch commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705261799
Hey, I'm noticing reverts are coming in fast without opportunities to push forward. I'm wondering if we should change the approach or the revert guidelines https://github.com/apache/incubator-superset/blob/master/CONTRIBUTING.md#revert-guidelines
I'm fine either way personally, but let's agree on guidelines.
----------------------------------------------------------------
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] mistercrunch commented on pull request #11194: fix: Revert "refactor: Replace react-bootstrap tabs with Antd tabs (#11090)"
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #11194:
URL: https://github.com/apache/incubator-superset/pull/11194#issuecomment-705672458
> Availability of the PR author: If the original PR author or the engineer who merged the code is highly available and can provide a fix in a reasonable timeframe, this would counter-indicate reverting.
That's the one item here, can we leave a minimum of ~24H after identifying the issue and reaching out to the author/merger combo on Slack?
----------------------------------------------------------------
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