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 2022/04/11 14:51:12 UTC
[GitHub] [superset] jsoref commented on a diff in pull request #19648: chore: Spelling
jsoref commented on code in PR #19648:
URL: https://github.com/apache/superset/pull/19648#discussion_r847371946
##########
CONTRIBUTING.md:
##########
@@ -661,7 +661,7 @@ We use [Pylint](https://pylint.org/) for linting which can be invoked via:
tox -e pylint
```
-In terms of best practices please advoid blanket disablement of Pylint messages globally (via `.pylintrc`) or top-level within the file header, albeit there being a few exceptions. Disablement should occur inline as it prevents masking issues and provides context as to why said message is disabled.
+In terms of best practices please avoid blanket disablement of Pylint messages globally (via `.pylintrc`) or top-level within the file header, albeit there being a few exceptions. Disablement should occur inline as it prevents masking issues and provides context as to why said message is disabled.
Review Comment:
Note: this PR is generally limited to spelling (including duplicate words). Grammatically, `disabling` would be better than `disablement`.
##########
RELEASING/release-notes-1-2/README.md:
##########
@@ -87,7 +87,7 @@ Expanding the API has been an ongoing effort, and 1.2 introduces several new API
- [14461](https://github.com/apache/superset/pull/14461) feat(native-filters): Auto apply changes in FiltersConfigModal (#14461) (@simcha90)
- [13507](https://github.com/apache/superset/pull/13507) feat(native-filters): Filter set tabs (#13507) (@simcha90)
- [14313](https://github.com/apache/superset/pull/14313) feat(native-filters): Implement adhoc filters and time picker in Range and Select native filters (#14313) (@Kamil Gabryjelski)
-- [14261](https://github.com/apache/superset/pull/14261) feat(native-filters): Show/Hide filter bar by metdata ff (#14261) (@simcha90)
+- [14261](https://github.com/apache/superset/pull/14261) feat(native-filters): Show/Hide filter bar by metadata ff (#14261) (@simcha90)
Review Comment:
Some projects don't like changing release notes. Let me know if this is such a project, it's trivial for me to ignore files/paths.
##########
docs/docs/miscellaneous/chart-params.mdx:
##########
@@ -18,7 +18,7 @@ Note not all fields are correctly categorized. The fields vary based on visualiz
| Field | Type | Notes |
| ----------------- | -------- | ----------------------------------- |
| `database_name` | _string_ | _Deprecated?_ |
-| `datasource` | _string_ | `<datasouce_id>__<datasource_type>` |
+| `datasource` | _string_ | `<datasource_id>__<datasource_type>` |
Review Comment:
I can reset table widths, but I'd rather wait until other basics are agreed upon before I do it.
##########
helm/superset/Chart.yaml:
##########
@@ -22,7 +22,7 @@ maintainers:
- name: craig-rueda
email: craig@craigrueda.com
url: https://github.com/craig-rueda
-version: 0.5.11
+version: 0.5.12
Review Comment:
The linter made me do this
##########
superset-frontend/packages/superset-ui-core/src/query/normalizeOrderBy.ts:
##########
@@ -56,7 +56,7 @@ export default function normalizeOrderBy(
};
}
- // todo: Removed `legacy_ordery_by` after refactoring
+ // todo: Removed `legacy_order_by` after refactoring
if (
queryObject.legacy_order_by !== undefined &&
queryObject.legacy_order_by !== null &&
Review Comment:
I checked and this was committed as a set, it looks like it was a combination of `order`+`by` twice, but w/ the final decision to be having a `_` between the words.
##########
superset-frontend/packages/superset-ui-core/src/number-format/README.md:
##########
@@ -68,7 +68,7 @@ There is also a formatter based on [pretty-ms](https://www.npmjs.com/package/pre
used to format time durations:
```js
-import { createDurationFormatter, formatNumber, getNumberFormatterRegistry } from from '@superset-ui-number-format';
+import { createDurationFormatter, formatNumber, getNumberFormatterRegistry } from '@superset-ui-number-format';
Review Comment:
This is an important flag of the duplicate word item, because it's in user facing documentation, and the documentation is wrong (but it isn't checked because no compiler checks code samples in docs).
##########
superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx:
##########
@@ -112,7 +112,7 @@ fetchMock.get('glob:*/api/v1/dashboard/26', {
json_metadata: mockedJsonMetadata,
owners: [],
position_json:
- '{"CHART-63bEuxjDMJ": {"children": [], "id": "CHART-63bEuxjDMJ", "meta": {"chartId": 369, "height": 76, "sliceName": "Vaccine Candidates per Country", "sliceNameOverride": "Map of Vaccine Candidates", "uuid": "ddc91df6-fb40-4826-bdca-16b85af1c024", "width": 7}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zvw7luvEL"], "type": "CHART"}, "CHART-F-fkth0Dnv": {"children": [], "id": "CHART-F-fkth0Dnv", "meta": {"chartId": 314, "height": 76, "sliceName": "Vaccine Candidates per Country", "sliceNameOverride": "Treemap of Vaccine Candidates per Country", "uuid": "e2f5a8a7-feb0-4f79-bc6b-01fe55b98b3c", "width": 5}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zvw7luvEL"], "type": "CHART"}, "CHART-RjD_ygqtwH": {"children": [], "id": "CHART-RjD_ygqtwH", "meta": {"chartId": 351, "height": 59, "sliceName": "Vaccine Candidates per Phase", "sliceNameOverride": "Vaccine Candidates per Phase", "uuid": "30b73c65-85e7-455f-bb24-801bb0cdc670", "width": 2}
, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-aGfmWtliqA": {"children": [], "id": "CHART-aGfmWtliqA", "meta": {"chartId": 312, "height": 59, "sliceName": "Vaccine Candidates per Phase", "uuid": "392f293e-0892-4316-bd41-c927b65606a4", "width": 4}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-dCUpAcPsji": {"children": [], "id": "CHART-dCUpAcPsji", "meta": {"chartId": 325, "height": 82, "sliceName": "Vaccine Candidates per Country & Stage", "sliceNameOverride": "Heatmap of Countries & Clinical Stages", "uuid": "cd111331-d286-4258-9020-c7949a109ed2", "width": 4}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "CHART-eirDduqb1A": {"children": [], "id": "CHART-eirDduqb1A", "meta": {"chartId": 358, "height": 59, "sliceName": "Filtering Vaccines", "sliceNameOverride": "Filter Box of Vaccines", "uuid": "c29381ce-0e99-4cf3-bf0f-5
f55d6b94176", "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-fYo7IyvKZQ": {"children": [], "id": "CHART-fYo7IyvKZQ", "meta": {"chartId": 371, "height": 82, "sliceName": "Vaccine Candidates per Country & Stage", "sliceNameOverride": "Sunburst of Country & Clinical Stages", "uuid": "f69c556f-15fe-4a82-a8bb-69d5b6954123", "width": 5}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "CHART-j4hUvP5dDD": {"children": [], "id": "CHART-j4hUvP5dDD", "meta": {"chartId": 364, "height": 82, "sliceName": "Vaccine Candidates per Approach & Stage", "sliceNameOverride": "Heatmap of Aproaches & Clinical Stages", "uuid": "0c953c84-0c9a-418d-be9f-2894d2a2cee0", "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "DASHBOARD_VERSION_KEY": "v2", "GRID_ID": {"children": [], "id": "GRID_ID", "parents": ["ROOT_ID"], "type": "GRID"}, "HE
ADER_ID": {"id": "HEADER_ID", "meta": {"text": "COVID Vaccine Dashboard"}, "type": "HEADER"}, "MARKDOWN-VjQQ5SFj5v": {"children": [], "id": "MARKDOWN-VjQQ5SFj5v", "meta": {"code": "# COVID-19 Vaccine Dashboard\\n\\nEverywhere you look, you see negative news about COVID-19. This is to be expected; it\'s been a brutal year and this disease is no joke. This dashboard hopes to use visualization to inject some optimism about the coming return to normalcy we enjoyed before 2020! There\'s lots to be optimistic about:\\n\\n- the sheer volume of attempts to fund the R&D needed to produce and bring an effective vaccine to market\\n- the large number of countries involved in at least one vaccine candidate (and the diversity of economic status of these countries)\\n- the diversity of vaccine approaches taken\\n- the fact that 2 vaccines have already been approved (and a hundreds of thousands of patients have already been vaccinated)\\n\\n### The Dataset\\n\\nThis dashboard is powered by data ma
intained by the Millken Institute ([link to dataset](https://airtable.com/shrSAi6t5WFwqo3GM/tblEzPQS5fnc0FHYR/viwDBH7b6FjmIBX5x?blocks=bipZFzhJ7wHPv7x9z)). We researched each vaccine candidate and added our own best guesses for the country responsible for each vaccine effort.\\n\\n_Note that this dataset was last updated on 12/23/2020_.\\n\\n", "height": 59, "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "MARKDOWN"}, "ROOT_ID": {"children": ["TABS-wUKya7eQ0Z"], "id": "ROOT_ID", "type": "ROOT"}, "ROW-xSeNAspgw": {"children": ["MARKDOWN-VjQQ5SFj5v", "CHART-aGfmWtliqA", "CHART-RjD_ygqtwH", "CHART-eirDduqb1A"], "id": "ROW-xSeNAspgw", "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "ROW-zhOlQLQnB": {"children": ["CHART-j4hUvP5dDD", "CHART-dCUpAcPsji", "CHART-fYo7IyvKZQ"], "id": "ROW-zhOlQLQnB", "meta": {"0": "ROOT_ID", "background": "BACKGROUN
D_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "ROW-zvw7luvEL": {"children": ["CHART-63bEuxjDMJ", "CHART-F-fkth0Dnv"], "id": "ROW-zvw7luvEL", "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "TAB-BCIJF4NvgQ": {"children": ["ROW-xSeNAspgw", "ROW-zvw7luvEL", "ROW-zhOlQLQnB"], "id": "TAB-BCIJF4NvgQ", "meta": {"text": "Overview"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z"], "type": "TAB"}, "TABS-wUKya7eQ0Z": {"children": ["TAB-BCIJF4NvgQ"], "id": "TABS-wUKya7eQ0Z", "meta": {}, "parents": ["ROOT_ID"], "type": "TABS"}}',
+ '{"CHART-63bEuxjDMJ": {"children": [], "id": "CHART-63bEuxjDMJ", "meta": {"chartId": 369, "height": 76, "sliceName": "Vaccine Candidates per Country", "sliceNameOverride": "Map of Vaccine Candidates", "uuid": "ddc91df6-fb40-4826-bdca-16b85af1c024", "width": 7}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zvw7luvEL"], "type": "CHART"}, "CHART-F-fkth0Dnv": {"children": [], "id": "CHART-F-fkth0Dnv", "meta": {"chartId": 314, "height": 76, "sliceName": "Vaccine Candidates per Country", "sliceNameOverride": "Treemap of Vaccine Candidates per Country", "uuid": "e2f5a8a7-feb0-4f79-bc6b-01fe55b98b3c", "width": 5}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zvw7luvEL"], "type": "CHART"}, "CHART-RjD_ygqtwH": {"children": [], "id": "CHART-RjD_ygqtwH", "meta": {"chartId": 351, "height": 59, "sliceName": "Vaccine Candidates per Phase", "sliceNameOverride": "Vaccine Candidates per Phase", "uuid": "30b73c65-85e7-455f-bb24-801bb0cdc670", "width": 2}
, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-aGfmWtliqA": {"children": [], "id": "CHART-aGfmWtliqA", "meta": {"chartId": 312, "height": 59, "sliceName": "Vaccine Candidates per Phase", "uuid": "392f293e-0892-4316-bd41-c927b65606a4", "width": 4}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-dCUpAcPsji": {"children": [], "id": "CHART-dCUpAcPsji", "meta": {"chartId": 325, "height": 82, "sliceName": "Vaccine Candidates per Country & Stage", "sliceNameOverride": "Heatmap of Countries & Clinical Stages", "uuid": "cd111331-d286-4258-9020-c7949a109ed2", "width": 4}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "CHART-eirDduqb1A": {"children": [], "id": "CHART-eirDduqb1A", "meta": {"chartId": 358, "height": 59, "sliceName": "Filtering Vaccines", "sliceNameOverride": "Filter Box of Vaccines", "uuid": "c29381ce-0e99-4cf3-bf0f-5
f55d6b94176", "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "CHART"}, "CHART-fYo7IyvKZQ": {"children": [], "id": "CHART-fYo7IyvKZQ", "meta": {"chartId": 371, "height": 82, "sliceName": "Vaccine Candidates per Country & Stage", "sliceNameOverride": "Sunburst of Country & Clinical Stages", "uuid": "f69c556f-15fe-4a82-a8bb-69d5b6954123", "width": 5}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "CHART-j4hUvP5dDD": {"children": [], "id": "CHART-j4hUvP5dDD", "meta": {"chartId": 364, "height": 82, "sliceName": "Vaccine Candidates per Approach & Stage", "sliceNameOverride": "Heatmap of Approaches & Clinical Stages", "uuid": "0c953c84-0c9a-418d-be9f-2894d2a2cee0", "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-zhOlQLQnB"], "type": "CHART"}, "DASHBOARD_VERSION_KEY": "v2", "GRID_ID": {"children": [], "id": "GRID_ID", "parents": ["ROOT_ID"], "type": "GRID"}, "H
EADER_ID": {"id": "HEADER_ID", "meta": {"text": "COVID Vaccine Dashboard"}, "type": "HEADER"}, "MARKDOWN-VjQQ5SFj5v": {"children": [], "id": "MARKDOWN-VjQQ5SFj5v", "meta": {"code": "# COVID-19 Vaccine Dashboard\\n\\nEverywhere you look, you see negative news about COVID-19. This is to be expected; it\'s been a brutal year and this disease is no joke. This dashboard hopes to use visualization to inject some optimism about the coming return to normalcy we enjoyed before 2020! There\'s lots to be optimistic about:\\n\\n- the sheer volume of attempts to fund the R&D needed to produce and bring an effective vaccine to market\\n- the large number of countries involved in at least one vaccine candidate (and the diversity of economic status of these countries)\\n- the diversity of vaccine approaches taken\\n- the fact that 2 vaccines have already been approved (and a hundreds of thousands of patients have already been vaccinated)\\n\\n### The Dataset\\n\\nThis dashboard is powered by data m
aintained by the Millken Institute ([link to dataset](https://airtable.com/shrSAi6t5WFwqo3GM/tblEzPQS5fnc0FHYR/viwDBH7b6FjmIBX5x?blocks=bipZFzhJ7wHPv7x9z)). We researched each vaccine candidate and added our own best guesses for the country responsible for each vaccine effort.\\n\\n_Note that this dataset was last updated on 12/23/2020_.\\n\\n", "height": 59, "width": 3}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ", "ROW-xSeNAspgw"], "type": "MARKDOWN"}, "ROOT_ID": {"children": ["TABS-wUKya7eQ0Z"], "id": "ROOT_ID", "type": "ROOT"}, "ROW-xSeNAspgw": {"children": ["MARKDOWN-VjQQ5SFj5v", "CHART-aGfmWtliqA", "CHART-RjD_ygqtwH", "CHART-eirDduqb1A"], "id": "ROW-xSeNAspgw", "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "ROW-zhOlQLQnB": {"children": ["CHART-j4hUvP5dDD", "CHART-dCUpAcPsji", "CHART-fYo7IyvKZQ"], "id": "ROW-zhOlQLQnB", "meta": {"0": "ROOT_ID", "background": "BACKGROU
ND_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "ROW-zvw7luvEL": {"children": ["CHART-63bEuxjDMJ", "CHART-F-fkth0Dnv"], "id": "ROW-zvw7luvEL", "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z", "TAB-BCIJF4NvgQ"], "type": "ROW"}, "TAB-BCIJF4NvgQ": {"children": ["ROW-xSeNAspgw", "ROW-zvw7luvEL", "ROW-zhOlQLQnB"], "id": "TAB-BCIJF4NvgQ", "meta": {"text": "Overview"}, "parents": ["ROOT_ID", "TABS-wUKya7eQ0Z"], "type": "TAB"}, "TABS-wUKya7eQ0Z": {"children": ["TAB-BCIJF4NvgQ"], "id": "TABS-wUKya7eQ0Z", "meta": {}, "parents": ["ROOT_ID"], "type": "TABS"}}',
Review Comment:
```diff
-Aproaches
+Approaches
```
##########
superset-frontend/src/explore/components/Control.test.tsx:
##########
@@ -61,7 +61,7 @@ describe('Control', () => {
it('render null if type is not valid', () => {
render(
setup({
- type: 'UnkownControl',
+ type: 'UnknownControl',
Review Comment:
?
##########
superset-frontend/src/views/components/types.ts:
##########
@@ -19,7 +19,7 @@
import { NavBarProps, MenuObjectProps } from './Menu';
-export interface ExtentionConfigs {
+export interface ExtensionConfigs {
Review Comment:
API break
##########
docs/src/resources/data.js:
##########
@@ -106,7 +106,7 @@ export const Databases = [
{
title: 'SQLite',
href: 'https://www.sqlite.org/index.html',
- imgName: 'sqllite.png',
+ imgName: 'sqlite.png',
Review Comment:
I missed renaming this file, but it's a brand so it should have the name I'm selecting.
##########
RELEASING/release-notes-1-4/README.md:
##########
@@ -19,7 +19,7 @@ under the License.
# Release Notes for Superset 1.4
-Superset 1.4 focuses heavily on continuing to polish the core Superset experience. This release has a very very long list of fixes from across the community.
+Superset 1.4 focuses heavily on continuing to polish the core Superset experience. This release has a very long list of fixes from across the community.
Review Comment:
I'm happy to drop this, but I don't think it added any real value. (I left one really long duplicate item.)
##########
RELEASING/Dockerfile.from_local_tarball:
##########
@@ -24,7 +24,7 @@ ENV LANG=C.UTF-8 \
RUN apt-get update -y
-# Install dependencies to fix `curl https support error` and `elaying package configuration warning`
+# Install dependencies to fix `curl https support error` and `delaying package configuration warning`
Review Comment:
When I saw this item flagged I guessed `relaying`, I was wrong.
##########
superset/viz.py:
##########
@@ -449,7 +449,7 @@ def cache_key(self, query_obj: QueryObjectDict, **extra: Any) -> str:
"5 days ago" or "now").
The `extra` arguments are currently used by time shift queries, since
- different time shifts wil differ only in the `from_dttm`, `to_dttm`,
+ different time shifts will differ only in the `from_dttm`, `to_dttm`,
Review Comment:
Some projects actually have a `wil`, but I think this is just a typo
##########
UPDATING.md:
##########
@@ -25,7 +25,7 @@ assists people when migrating to a new version.
## Next
- [19046](https://github.com/apache/superset/pull/19046): Enables the drag and drop interface in Explore control panel by default. Flips `ENABLE_EXPLORE_DRAG_AND_DROP` and `ENABLE_DND_WITH_CLICK_UX` feature flags to `True`.
-- [18936](https://github.com/apache/superset/pull/18936): Removes legacy SIP-15 interm logic/flags—specifically the `SIP_15_ENABLED`, `SIP_15_GRACE_PERIOD_END`, `SIP_15_DEFAULT_TIME_RANGE_ENDPOINTS`, and `SIP_15_TOAST_MESSAGE` flags. Time range endpoints are no longer configurable and strictly adhere to the `[start, end)` paradigm, i.e., inclusive of the start and exclusive of the end. Additionally this change removes the now obsolete `time_range_endpoints` from the form-data and resulting in the cache being busted.
+- [18936](https://github.com/apache/superset/pull/18936): Removes legacy SIP-15 interim logic/flags—specifically the `SIP_15_ENABLED`, `SIP_15_GRACE_PERIOD_END`, `SIP_15_DEFAULT_TIME_RANGE_ENDPOINTS`, and `SIP_15_TOAST_MESSAGE` flags. Time range endpoints are no longer configurable and strictly adhere to the `[start, end)` paradigm, i.e., inclusive of the start and exclusive of the end. Additionally this change removes the now obsolete `time_range_endpoints` from the form-data and resulting in the cache being busted.
Review Comment:
?
##########
superset-frontend/packages/superset-ui-core/src/math-expression/index.ts:
##########
@@ -106,7 +106,7 @@ export function evalExpression(expression: string, value: number): number {
const subExpressions = String(parsedExpression).split('=');
parsedExpression = subExpressions[1] ?? subExpressions[0];
// we can ignore the type requirement on `TOKENS`, as value is always `number`
- // and doesn't need to consider `number | underfined`.
+ // and doesn't need to consider `number | undefined`.
Review Comment:
probably?
##########
RELEASING/changelog.py:
##########
@@ -24,9 +24,9 @@
from click.core import Context
try:
- from github import BadCredentialsException, Github, PullRequest, Repository
+ from github import BadCredentialsException, GitHub, PullRequest, Repository
except ModuleNotFoundError:
- print("PyGithub is a required package for this script")
+ print("PyGitHub is a required package for this script")
exit(1)
Review Comment:
If these are third party modules, then these changes will break. It's trivial for me to drop individual change families although I'll probably try to carefully tease out the text and noncode instances of GitHub.
##########
superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts:
##########
@@ -27,7 +27,7 @@ interface QueryString {
native_filters_key: string;
}
-xdescribe('nativefiler url param key', () => {
+xdescribe('nativefilter url param key', () => {
Review Comment:
?
##########
tests/integration_tests/event_logger_tests.py:
##########
@@ -179,7 +179,7 @@ def log(
duration=timedelta(days=64, seconds=29156, microseconds=10),
object_ref={"baz": "food"},
log_to_statsd=False,
- payload_override={"engine": "sqllite"},
+ payload_override={"engine": "sqlite"},
Review Comment:
?
##########
CODE_OF_CONDUCT.md:
##########
@@ -119,7 +119,7 @@ If you decide to join the [Community Slack](https://join.slack.com/t/apache-supe
**3. Ask thoughtful questions.**
-- We’re all here to help each other out. The best way to get help is by investing effort into your questions. First check and see if your question is answered in [the Superset documentation](https://superset.apache.org/faq.html) or on [Stack Overflow](https://stackoverflow.com/search?q=apache+superset). You can also check [Github issues](https://github.com/apache/superset/issues) to see if your question or feature request has been submitted before. Then, use Slack search to see if your question has already been asked and answered in the past. If you still feel the need to ask a question, make sure you include:
+- We’re all here to help each other out. The best way to get help is by investing effort into your questions. First check and see if your question is answered in [the Superset documentation](https://superset.apache.org/faq.html) or on [Stack Overflow](https://stackoverflow.com/search?q=apache+superset). You can also check [GitHub issues](https://github.com/apache/superset/issues) to see if your question or feature request has been submitted before. Then, use Slack search to see if your question has already been asked and answered in the past. If you still feel the need to ask a question, make sure you include:
Review Comment:
Brand
##########
UPDATING.md:
##########
@@ -221,7 +221,7 @@ flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in conf
- [10324](https://github.com/apache/superset/pull/10324): Facebook Prophet has been introduced as an optional dependency to add support for timeseries forecasting in the chart data API. To enable this feature, install Superset with the optional dependency `prophet` or directly `pip install fbprophet`.
-- [10320](https://github.com/apache/superset/pull/10320): References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`.
+- [10320](https://github.com/apache/superset/pull/10320): References to blacklist/whitelist language have been replaced with more appropriate alternatives. All configs referencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`.
Review Comment:
I appreciate the improvements, but I don't think the typo here was intended to avoid including one instance given that there are a number of the term in this line.
##########
superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/legacy-preset-chart-nvd3/Area/stories/controlsShown.tsx:
##########
@@ -31,7 +31,7 @@ export const controlsShown = () => (
queriesData={[{ data }]}
formData={{
bottomMargin: 'auto',
- colorCcheme: 'd3Category10',
+ colorScheme: 'd3Category10',
Review Comment:
This is interesting
##########
docs/docs/installation/configuring-superset.mdx:
##########
@@ -99,7 +99,7 @@ If you're not using Gunicorn, you may want to disable the use of `flask-compress
If you are running superset behind a load balancer or reverse proxy (e.g. NGINX or ELB on AWS), you
may need to utilize a healthcheck endpoint so that your load balancer knows if your superset
instance is running. This is provided at `/health` which will return a 200 response containing “OK”
-if the the webserver is running.
+if the webserver is running.
Review Comment:
This is a doubled word, it's a fairly new feature of my tooling, and I'm generally fairly happy w/ the items it flags.
##########
superset-frontend/plugins/legacy-plugin-chart-parallel-coordinates/src/ReactParallelCoordinates.jsx:
##########
@@ -23,17 +23,17 @@ import Component from './ParallelCoordinates';
const ReactComponent = reactify(Component);
-const ParallelCoordianes = ({ className, ...otherProps }) => (
+const ParallelCoordinates = ({ className, ...otherProps }) => (
<div className={className}>
<ReactComponent {...otherProps} />
</div>
);
-ParallelCoordianes.propTypes = {
+ParallelCoordinates.propTypes = {
className: PropTypes.string.isRequired,
};
-export default styled(ParallelCoordianes)`
+export default styled(ParallelCoordinates)`
Review Comment:
API change?
##########
superset-frontend/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx:
##########
@@ -110,7 +110,7 @@ export const fixedHeight100Width = () => {
};
fixedHeight100Width.story = { name: 'fixed height, 100% width' };
-export const withErrorBoundar = () => {
+export const withErrorBoundary = () => {
Review Comment:
API break
##########
superset-frontend/packages/superset-ui-core/src/query/types/AdvancedAnalytics.ts:
##########
@@ -29,15 +29,15 @@ export interface RollingWindow {
min_periods?: number;
}
-export enum ComparisionType {
+export enum ComparisonType {
Values = 'values',
Difference = 'difference',
Percentage = 'percentage',
Ratio = 'ratio',
}
export interface TimeCompare {
time_compare?: string;
- comparison_type?: ComparisionType;
+ comparison_type?: ComparisonType;
Review Comment:
This is an API break
##########
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx:
##########
@@ -93,7 +93,7 @@ describe('plugin-chart-table', () => {
<TableChart {...transformProps(testData.advanced)} sticky={false} />,
);
tree = wrap.render();
- // should successfull rerender with new props
+ // should successful rerender with new props
Review Comment:
This should probably be successfully
##########
superset-frontend/src/preamble.ts:
##########
@@ -25,7 +25,7 @@ import { merge } from 'lodash';
import setupClient from './setup/setupClient';
import setupColors from './setup/setupColors';
import setupFormatters from './setup/setupFormatters';
-import setupDashboardComponents from './setup/setupDasboardComponents';
+import setupDashboardComponents from './setup/setupDashboardComponents';
Review Comment:
Renamed below:
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/utils/fitViewport.ts:
##########
@@ -21,7 +21,7 @@ import computeBoundsFromPoints from './computeBoundsFromPoints';
import { Point } from '../types';
export type Viewport = {
- longtitude: number;
+ longitude: number;
Review Comment:
API change?
##########
superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts:
##########
@@ -75,7 +75,7 @@ interface PreselectedFilterColumn {
[key: string]: boolean | string | number | string[] | number[];
}
-interface PreselectedFiltersMeatadata {
+interface PreselectedFiltersMetadata {
Review Comment:
API break?
##########
superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx:
##########
@@ -166,7 +166,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
const uploadDropdownMenu = [
{
label: t('Upload file to database'),
- childs: [
+ children: [
Review Comment:
Probably an api break.
##########
superset/cli/importexport.py:
##########
@@ -66,7 +66,7 @@ def import_directory(directory: str, overwrite: bool, force: bool) -> None:
@click.option(
"--dashboard-file",
"-f",
- help="Specify the the file to export to",
+ help="Specify the file to export to",
Review Comment:
This is public facing of some sort
##########
superset/dashboards/filter_sets/commands/base.py:
##########
@@ -20,7 +20,7 @@
from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
-from superset.common.not_authrized_object import NotAuthorizedException
+from superset.common.not_authorized_object import NotAuthorizedException
Review Comment:
This file was renamed above
##########
superset/connectors/sqla/models.py:
##########
@@ -1158,7 +1158,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
if metrics_exprs:
main_metric_expr = metrics_exprs[0]
else:
- main_metric_expr, label = literal_column("COUNT(*)"), "ccount"
+ main_metric_expr, label = literal_column("COUNT(*)"), "count"
Review Comment:
This is a notable change. I'm not certain about it.
##########
superset/examples/configs/datasets/examples/FCC_2018_Survey.yaml:
##########
@@ -930,7 +930,7 @@ columns:
expression: null
description: null
python_date_format: null
-- column_name: rsrc_pluralsght
+- column_name: rsrc_pluralsight
Review Comment:
At a certain point, I decided not to rename most of the `column_name` fields from files like this.
I'm not sure if I should/shouldn't.
##########
superset/examples/configs/charts/Messages_per_Channel.yaml:
##########
@@ -50,7 +50,7 @@ params:
dashboards: '#3CCCCB'
design: '#1FA8C9'
developers: '#9EE5E5'
- embedd-dashboards: '#ACE1C4'
+ embedded-dashboards: '#ACE1C4'
Review Comment:
I can't tell if this should be `embed-dashboards` or `embedded-dashboards`. I think I'm right.
##########
superset/migrations/versions/4736ec66ce19_.py:
##########
@@ -124,7 +124,7 @@ def upgrade():
logging.warning(
"Constraint drop failed, you may want to do this "
"manually on your database. For context, this is a known "
- "issue around undeterministic contraint names on Postgres "
+ "issue around nondeterministic constraint names on Postgres "
Review Comment:
Notable change
##########
superset/reports/schemas.py:
##########
@@ -84,7 +84,7 @@
"Superset nags you again. (in seconds)"
)
working_timeout_description = (
- "If an alert is staled at a working state, how long until it's state is reseted to"
+ "If an alert is staled at a working state, how long until its state is reset to"
Review Comment:
As noted earlier, I'm generally not dealing w/ grammar, I made this change to `it's` because I was going to make the change to `reset` and my tooling cares about order. I have not checked the rest of the repo for `it's`/`its`, but i'm sure there are plenty of instances where it's missued, misuse of its is fairly comment everywhere.
##########
superset/examples/multiformat_time_series.py:
##########
@@ -97,7 +97,7 @@ def load_multiformat_time_series( # pylint: disable=too-many-locals
for col in obj.columns:
dttm_and_expr = dttm_and_expr_dict[col.column_name]
col.python_date_format = dttm_and_expr[0]
- col.dbatabase_expr = dttm_and_expr[1]
+ col.database_expr = dttm_and_expr[1]
Review Comment:
Not sure about this.
Possibly an api break
##########
superset/utils/core.py:
##########
@@ -128,7 +128,7 @@
NO_TIME_RANGE = "No filter"
-TIME_COMPARISION = "__"
+TIME_COMPARISON = "__"
Review Comment:
API change
##########
superset/utils/core.py:
##########
@@ -332,7 +332,7 @@ class TemporalType(str, Enum):
class ColumnTypeSource(Enum):
GET_TABLE = 1
- CURSOR_DESCRIPION = 2
+ CURSOR_DESCRIPTION = 2
Review Comment:
API change
##########
tests/integration_tests/annotation_layers/fixtures.py:
##########
@@ -73,7 +73,7 @@ def _insert_annotation(
def create_annotation_layers():
"""
Creates ANNOTATION_LAYERS_COUNT-1 layers with no annotations
- and a final one with ANNOTATION_COUNT childs
+ and a final one with ANNOTATION_COUNT children
Review Comment:
?
##########
superset/views/base_api.py:
##########
@@ -66,7 +66,7 @@ class DistinctResultResponseSchema(Schema):
text = fields.String(description="The distinct item")
-class DistincResponseSchema(Schema):
+class DistinctResponseSchema(Schema):
Review Comment:
API break
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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