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/26 04:22:39 UTC
[GitHub] [incubator-superset] rusackas opened a new pull request #9389:
Migrating unique BigNumber(/Total) controls
rusackas opened a new pull request #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389
### 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/BigNumber.js` and `/explore/controlPanels/BigNumberTotal.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
@kristw
----------------------------------------------------------------
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
#9389: Migrating unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#discussion_r398926207
##########
File path: CONTRIBUTING.md
##########
@@ -1016,15 +1016,6 @@ The `metric` (or equivalent) and `timeseries_limit_metric` fields are all compos
The filter-box configuration references column names (via the `column` key) and optionally metric names (via the `metric` key) if sorting is defined.
-### Options
-
-| Field | Type | Notes |
-| ---------------------- | --------- | ------------------------------------ |
-| `compare_lag` | _number_ | The **Comparison Period Lag** widget |
-| `compare_suffix` | _string_ | The **Comparison suffix** widget |
-| `show_trend_line` | _boolean_ | The **Show Trend Line** widget |
-| `start_y_axis_at_zero` | _boolean_ | The **Start y-axis at 0** widget |
-
Review comment:
I am not 100% sure about this part but seems reasonable I guess. Or you can copy the note as comment in the js file.
----------------------------------------------------------------
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
#9389: Migrating unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#discussion_r398310221
##########
File path: CONTRIBUTING.md
##########
@@ -1016,15 +1016,6 @@ The `metric` (or equivalent) and `timeseries_limit_metric` fields are all compos
The filter-box configuration references column names (via the `column` key) and optionally metric names (via the `metric` key) if sorting is defined.
-### Options
-
-| Field | Type | Notes |
-| ---------------------- | --------- | ------------------------------------ |
-| `compare_lag` | _number_ | The **Comparison Period Lag** widget |
-| `compare_suffix` | _string_ | The **Comparison suffix** widget |
-| `show_trend_line` | _boolean_ | The **Show Trend Line** widget |
-| `start_y_axis_at_zero` | _boolean_ | The **Start y-axis at 0** widget |
-
Review comment:
@kristw I'll just keep removing these things as the controls are moved out of `controls.js`, but let me know if you have thoughts on when/where NOT to do that, so I don't kill any useful documentation.
----------------------------------------------------------------
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 issue #9389: Migrating
unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#issuecomment-607553485
Closing in favor of #9440
----------------------------------------------------------------
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 issue #9389: Migrating
unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#issuecomment-607393397
Not really sure why CI is failing on this one... I just restarted it to see if I get lucky ;)
Happy to rebase (or even re-do) these changes on an updated `master`
----------------------------------------------------------------
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 edited a comment on issue #9389:
Migrating unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
rusackas edited a comment on issue #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#issuecomment-607393397
Not really sure why CI is failing on this one (was actually just starting to look into it and try reproducing locally)... I just restarted Travis to see if I get lucky ;)
Happy to rebase (or even re-do) these changes on an updated `master`
----------------------------------------------------------------
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
#9389: Migrating unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#discussion_r398926207
##########
File path: CONTRIBUTING.md
##########
@@ -1016,15 +1016,6 @@ The `metric` (or equivalent) and `timeseries_limit_metric` fields are all compos
The filter-box configuration references column names (via the `column` key) and optionally metric names (via the `metric` key) if sorting is defined.
-### Options
-
-| Field | Type | Notes |
-| ---------------------- | --------- | ------------------------------------ |
-| `compare_lag` | _number_ | The **Comparison Period Lag** widget |
-| `compare_suffix` | _string_ | The **Comparison suffix** widget |
-| `show_trend_line` | _boolean_ | The **Show Trend Line** widget |
-| `start_y_axis_at_zero` | _boolean_ | The **Start y-axis at 0** widget |
-
Review comment:
I am not 100% sure about this part but seems reasonable I guess. Or you can copy the note as comment in the js file if it seems useful.
----------------------------------------------------------------
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 closed pull request #9389: Migrating
unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
rusackas closed pull request #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389
----------------------------------------------------------------
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 issue #9389: Migrating
unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
kristw commented on issue #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#issuecomment-607392237
I'll merge the other one first then since this one's ci is still broken.
----------------------------------------------------------------
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] ktmud commented on issue #9389: Migrating
unique BigNumber(/Total) controls
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #9389: Migrating unique BigNumber(/Total) controls
URL: https://github.com/apache/incubator-superset/pull/9389#issuecomment-607385360
I also made change to the Big Number controls here: https://github.com/apache/incubator-superset/pull/9341
It seems there will be conflicts. Which one should we merge first?
----------------------------------------------------------------
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