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/03/24 22:16:22 UTC
[GitHub] [incubator-superset] rusackas opened a new pull request #9373:
Migrating mapbox controls
rusackas opened a new pull request #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373
### CATEGORY
Choose one
- [ ] Bug Fix
- [ ] Enhancement (new features, refinement)
- [x] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Moving unique control(s) from `controls.jsx` to `/explore/controlPanels/Mapbox.js`
### 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 -->
- [x] Has associated issue: #9187
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#9373: Migrating mapbox controls
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373#discussion_r398927573
##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -191,7 +191,7 @@ const jsFunctionInfo = (
</div>
);
-function columnChoices(datasource) {
+export function columnChoices(datasource) {
Review comment:
Agree. We can separate this function into its own file starting in this PR too.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas merged pull request #9373: Migrating
mapbox controls
Posted by GitBox <gi...@apache.org>.
rusackas merged pull request #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#9373: Migrating mapbox controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373#discussion_r399029926
##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -191,7 +191,7 @@ const jsFunctionInfo = (
</div>
);
-function columnChoices(datasource) {
+export function columnChoices(datasource) {
Review comment:
I'll make another PR moving this (and probably some other things) into separate files/modules. I'll track it in my internal documents for now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#9373: Migrating mapbox controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373#discussion_r397495315
##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -191,7 +191,7 @@ const jsFunctionInfo = (
</div>
);
-function columnChoices(datasource) {
+export function columnChoices(datasource) {
Review comment:
@kristw just pointing out this export/import... we may want to move this to `utils` or somewhere else that makes more sense.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #9373: Migrating
mapbox controls
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9373: Migrating mapbox controls
URL: https://github.com/apache/incubator-superset/pull/9373#issuecomment-604635255
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9373?src=pr&el=h1) Report
> Merging [#9373](https://codecov.io/gh/apache/incubator-superset/pull/9373?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cfa786850f337353578e45470c473cea3ca35c89&el=desc) will **not change** coverage by `%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9373?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9373 +/- ##
=======================================
Coverage 59.01% 59.01%
=======================================
Files 376 377 +1
Lines 12162 12162
Branches 3004 3004
=======================================
Hits 7177 7177
Misses 4805 4805
Partials 180 180
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9373?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/explore/controlPanels/Mapbox.js](https://codecov.io/gh/apache/incubator-superset/pull/9373/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy9NYXBib3guanM=) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9373/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `39.09% <0.00%> (+0.29%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9373?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/9373?src=pr&el=footer). Last update [cfa7868...aac5c36](https://codecov.io/gh/apache/incubator-superset/pull/9373?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org