You are viewing a plain text version of this content. The canonical link for it is here.
Posted to devnull@infra.apache.org by "kristw (GitHub)" <gi...@apache.org> on 2018/08/23 00:54:53 UTC
[GitHub] [incubator-superset] kristw opened pull request #5707:
Refactor table
- Extract `slice` and `formData` according to [SIP-5]
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #5707:
Refactor table
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=h1) Report
> Merging [#5707](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/89927558e85cd121b46818cb2f38d3a2e61e25d6?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5707 +/- ##
==========================================
- Coverage 63.45% 63.43% -0.02%
==========================================
Files 361 361
Lines 22972 22975 +3
Branches 2557 2560 +3
==========================================
- Hits 14576 14574 -2
- Misses 8381 8386 +5
Partials 15 15
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/assets/src/chart/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9DaGFydC5qc3g=) | `58.13% <ø> (+0.67%)` | :arrow_up: |
| [superset/assets/src/visualizations/table.js](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aXN1YWxpemF0aW9ucy90YWJsZS5qcw==) | `0% <0%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?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/5707?src=pr&el=footer). Last update [8992755...9421da8](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster commented on pull request
#5707: [SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
"Pn" means positive/negative? could we update that variable name to be more descriptive to indicate that?
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster commented on pull request
#5707: [SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
I would guess that this tried to remove tooltips from other dashboard charts 🤔
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #5707:
[SIP-5] Refactor table
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=h1) Report
> Merging [#5707](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/f72cdc38dfcc2d1bf5574a8e0204927090225720?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `90.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5707 +/- ##
==========================================
+ Coverage 63.75% 63.78% +0.03%
==========================================
Files 364 364
Lines 23104 23110 +6
Branches 2579 2587 +8
==========================================
+ Hits 14729 14740 +11
+ Misses 8360 8355 -5
Partials 15 15
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/assets/src/chart/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9DaGFydC5qc3g=) | `58.4% <ø> (+0.7%)` | :arrow_up: |
| [superset/assets/src/visualizations/table.js](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aXN1YWxpemF0aW9ucy90YWJsZS5qcw==) | `85.82% <90.14%> (+3.06%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?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/5707?src=pr&el=footer). Last update [f72cdc3...1d034cf](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #5707:
[SIP-5] Refactor table
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=h1) Report
> Merging [#5707](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/f72cdc38dfcc2d1bf5574a8e0204927090225720?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `90.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5707 +/- ##
==========================================
+ Coverage 63.75% 63.78% +0.03%
==========================================
Files 364 364
Lines 23104 23110 +6
Branches 2579 2587 +8
==========================================
+ Hits 14729 14740 +11
+ Misses 8360 8355 -5
Partials 15 15
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/assets/src/chart/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9DaGFydC5qc3g=) | `58.4% <ø> (+0.7%)` | :arrow_up: |
| [superset/assets/src/visualizations/table.js](https://codecov.io/gh/apache/incubator-superset/pull/5707/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aXN1YWxpemF0aW9ucy90YWJsZS5qcw==) | `85.82% <90.14%> (+3.06%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/5707?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/5707?src=pr&el=footer). Last update [f72cdc3...1d034cf](https://codecov.io/gh/apache/incubator-superset/pull/5707?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster closed pull request #5707:
[SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
[ pull request closed by williaster ]
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] hughhhh commented on pull request
#5707: Refactor table
Posted by "hughhhh (GitHub)" <gi...@apache.org>.
Why do we need a No Op on these prop functions?
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster commented on pull request
#5707: [SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
I'm not sure if it'd be useful to put this into the `spec/helpers/browser.js` file, that's where other things like this currently are. but once we move to packages, this type of config should be specific to each package.
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] kristw commented on pull request #5707:
Refactor table
Posted by "kristw (GitHub)" <gi...@apache.org>.
To avoid checking if `onAddFilter` exists every time before calling it.
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] kristw commented on pull request #5707:
Refactor table
Posted by "kristw (GitHub)" <gi...@apache.org>.
Set `NOOP` as default value to avoid checking if `onAddFilter` exists every time before calling it.
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster commented on pull request
#5707: [SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
that's interesting. I think this table used to be able to function as a filter on dashboards but perhaps that broke at some point. @mistercrunch any thoughts?
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] kristw commented on issue #5707:
[SIP-5] Refactor table
Posted by "kristw (GitHub)" <gi...@apache.org>.
Addressed comments, except the `tableFilter` one.
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [incubator-superset] williaster commented on pull request
#5707: [SIP-5] Refactor table
Posted by "williaster (GitHub)" <gi...@apache.org>.
possibly good news: anything with `.widget` I think can be removed. this is what grid cells on the old dashboard used to be called.
[ Full content available at: https://github.com/apache/incubator-superset/pull/5707 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org