This is an automated email from the ASF dual-hosted git repository.
kristw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 5ba7fca docs: update CONTRIBUTING with TypeScript details from
[SIP-36] (#9185)
5ba7fca is described below
commit 5ba7fcaeea86413857d3618fdeb3c22b95196585
Author: Erik Ritter <[email protected]>
AuthorDate: Mon Feb 24 14:31:23 2020 -0800
docs: update CONTRIBUTING with TypeScript details from [SIP-36] (#9185)
---
CONTRIBUTING.md | 178 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 112 insertions(+), 66 deletions(-)
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index b1dd3f4..1d8571a 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -24,37 +24,76 @@ little bit helps, and credit will always be given.
## Table of Contents
-- [Orientation](#orientation)
-- [Types of Contributions](#types-of-contributions)
- - [Report Bugs](#report-bugs)
- - [Submit Ideas or Feature Requests](#submit-ideas-or-feature-requests)
- - [Ask Questions](#ask-questions)
- - [Fix Bugs](#fix-bugs)
- - [Implement Features](#implement-features)
- - [Improve Documentation](#improve-documentation)
- - [Add Translations](#add-translations)
-- [Pull Request Guidelines](#pull-request-guidelines)
- - [Protocol](#protocol)
-- [Managing Issues and PRs](#managing-issues-and-prs)
-- [Revert Guidelines](#revert-guidelines)
-- [Setup Local Environment for
Development](#setup-local-environment-for-development)
- - [Documentation](#documentation)
- - [Flask server](#flask-server)
- - [Frontend assets](#frontend-assets)
-- [Testing](#testing)
- - [JavaScript testing](#javascript-testing)
- - [Integration testing](#integration-testing)
+- [Contributing](#contributing)
+ - [Table of Contents](#table-of-contents)
+ - [Orientation](#orientation)
+ - [Types of Contributions](#types-of-contributions)
+ - [Report Bug](#report-bug)
+ - [Submit Ideas or Feature Requests](#submit-ideas-or-feature-requests)
+ - [Fix Bugs](#fix-bugs)
+ - [Implement Features](#implement-features)
+ - [Improve Documentation](#improve-documentation)
+ - [Add Translations](#add-translations)
+ - [Ask Questions](#ask-questions)
+ - [Pull Request Guidelines](#pull-request-guidelines)
+ - [Protocol](#protocol)
+ - [Authoring](#authoring)
+ - [Reviewing](#reviewing)
+ - [Merging](#merging)
+ - [Post-merge Responsibility](#post-merge-responsibility)
+ - [Managing Issues and PRs](#managing-issues-and-prs)
+ - [Revert Guidelines](#revert-guidelines)
+ - [Setup Local Environment for
Development](#setup-local-environment-for-development)
+ - [Documentation](#documentation)
+ - [Images](#images)
+ - [API documentation](#api-documentation)
+ - [Flask server](#flask-server)
+ - [OS Dependencies](#os-dependencies)
+ - [Logging to the browser console](#logging-to-the-browser-console)
+ - [Frontend Assets](#frontend-assets)
+ - [nvm and node](#nvm-and-node)
+ - [Prerequisite](#prerequisite)
+ - [Installing Dependencies](#installing-dependencies)
+ - [Building](#building)
+ - [Docker (docker-compose)](#docker-docker-compose)
+ - [Updating NPM packages](#updating-npm-packages)
+ - [Feature flags](#feature-flags)
+ - [Git Hooks](#git-hooks)
- [Linting](#linting)
-- [Translating](#translating)
- - [Enabling language selection](#enabling-language-selection)
- - [Extracting new strings for
translation](#extracting-new-strings-for-translation)
- - [Creating a new language dictionary](#creating-a-new-language-dictionary)
-- [Tips](#tips)
- - [Adding a new datasource](#adding-a-new-datasource)
- - [Creating a new visualization type](#creating-a-new-visualization-type)
- - [Adding a DB migration](#adding-a-db-migration)
- - [Merging DB migrations](#merging-db-migrations)
- - [SQL Lab Async](#sql-lab-async)
+ - [Conventions](#conventions)
+ - [Python](#python)
+ - [Typing](#typing)
+ - [Python](#python-1)
+ - [TypeScript](#typescript)
+ - [Testing](#testing)
+ - [Python Testing](#python-testing)
+ - [Frontend Testing](#frontend-testing)
+ - [Integration Testing](#integration-testing)
+ - [Translating](#translating)
+ - [Enabling language selection](#enabling-language-selection)
+ - [Extracting new strings for
translation](#extracting-new-strings-for-translation)
+ - [Creating a new language dictionary](#creating-a-new-language-dictionary)
+ - [Tips](#tips)
+ - [Adding a new datasource](#adding-a-new-datasource)
+ - [Creating a new visualization type](#creating-a-new-visualization-type)
+ - [Adding a DB migration](#adding-a-db-migration)
+ - [Merging DB migrations](#merging-db-migrations)
+ - [SQL Lab Async](#sql-lab-async)
+ - [Chart Parameters](#chart-parameters)
+ - [Datasource & Chart Type](#datasource--chart-type)
+ - [Time](#time)
+ - [GROUP BY](#group-by)
+ - [NOT GROUPED BY](#not-grouped-by)
+ - [Y Axis 1](#y-axis-1)
+ - [Y Axis 2](#y-axis-2)
+ - [Query](#query)
+ - [Filters Configuration](#filters-configuration)
+ - [Options](#options)
+ - [Chart Options](#chart-options)
+ - [X Axis](#x-axis)
+ - [Y Axis](#y-axis)
+ - [Other](#other)
+ - [Unclassified](#unclassified)
## Orientation
@@ -64,7 +103,7 @@ Here's a list of repositories that contain Superset-related
packages:
is the main repository containing the `apache-superset` Python package
distributed on
[pypi](https://pypi.org/project/apache-superset/). This repository
- also includes Superset's main Javascript bundles and react apps under
+ also includes Superset's main TypeScript/JavaScript bundles and react apps
under
the
[superset-frontend](https://github.com/apache/incubator-superset/tree/master/superset-frontend)
folder.
- [apache-superset/superset-ui](https://github.com/apache-superset/superset-ui)
@@ -165,7 +204,7 @@ Finally, never submit a PR that will put master branch in
broken state. If the P
- If no screenshot is provided, the committers will mark the PR with
`need:screenshot` label and will not review until screenshot is provided.
- **Dependencies:** Be careful about adding new dependency and avoid
unnecessary dependencies.
- For Python, include it in `setup.py` denoting any specific restrictions
and in `requirements.txt` pinned to a specific version which ensures that the
application build is deterministic.
- - For Javascript, include new libraries in `package.json`
+ - For TypeScript/JavaScript, include new libraries in `package.json`
- **Tests:** The pull request should include tests, either as doctests, unit
tests, or both. Make sure to resolve all errors and test failures. See
[Testing](#testing) for how to run tests.
- **Documentation:** If the pull request adds functionality, the docs should
be updated as part of the same PR. Doc string are often sufficient, make sure
to follow the sphinx compatible standards.
- **CI:** Reviewers will not review the code until all CI tests are passed.
Sometimes there can be flaky tests. You can close and open PR to re-run CI
test. Please report if the issue persists. After the CI fix has been deployed
to `master`, please rebase your PR.
@@ -239,6 +278,7 @@ Reverting changes that are causing issues in the master
branch is a normal and e
- **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.
Should you decide that reverting is desirable, it is the responsibility of the
Contributor performing the revert to:
+
- **Contact the interested parties:** The PR's author and the engineer who
merged the work should both be contacted and informed of the revert.
- **Provide concise reproduction steps:** Ensure that the issue can be clearly
understood and duplicated by the original author of the PR.
- **Put the revert through code review:** The revert must be approved by
another committer.
@@ -418,7 +458,7 @@ app.logger.info(form_data)
### Frontend Assets
-Frontend assets (JavaScript, CSS, and images) must be compiled in order to
properly display the web UI. The `superset-frontend` directory contains all
NPM-managed front end assets. Note that there are additional frontend assets
bundled with Flask-Appbuilder (e.g. jQuery and bootstrap); these are not
managed by NPM, and may be phased out in the future.
+Frontend assets (TypeScript, JavaScript, CSS, and images) must be compiled in
order to properly display the web UI. The `superset-frontend` directory
contains all NPM-managed front end assets. Note that there are additional
frontend assets bundled with Flask-Appbuilder (e.g. jQuery and bootstrap);
these are not managed by NPM, and may be phased out in the future.
#### nvm and node
@@ -464,7 +504,7 @@ Alternatively you can use one of the following commands.
# Start a watcher that recompiles your assets as you modify them (but have to
manually reload your browser to see changes.)
npm run dev
-# Compile the Javascript and CSS in production/optimized mode for official
releases
+# Compile the TypeScript/JavaScript and CSS in production/optimized mode for
official releases
npm run prod
```
@@ -523,7 +563,7 @@ Lint the project with:
# for python
tox -e flake8
-# for javascript
+# for frontend
cd superset-frontend
npm ci
npm run lint
@@ -550,6 +590,39 @@ blueprints = app.config.get("BLUEPRINTS")
or similar as the later will cause typing issues. The former is of type
`List[Callable]` whereas the later is of type `Optional[List[Callable]]`.
+## Typing
+
+### Python
+
+To ensure clarity, consistency, all readability, _all_ new functions should use
+[type hints](https://docs.python.org/3/library/typing.html) and include a
+docstring using Sphinx documentation.
+
+Note per [PEP-484](https://www.python.org/dev/peps/pep-0484/#exceptions) no
+syntax for listing explicitly raised exceptions is proposed and thus the
+recommendation is to put this information in a docstring, i.e.,
+
+```python
+import math
+from typing import Union
+
+
+def sqrt(x: Union[float, int]) -> Union[float, int]:
+ """
+ Return the square root of x.
+
+ :param x: A number
+ :returns: The square root of the given number
+ :raises ValueError: If the number is negative
+ """
+
+ return math.sqrt(x)
+```
+
+### TypeScript
+
+TypeScript is fully supported and is the recommended language for writing all
new frontend components. When modifying existing functions/components,
migrating to TypeScript is appreciated, but not required. Examples of migrating
functions/components to TypeScript can be found in
[#9162](https://github.com/apache/incubator-superset/pull/9162) and
[#9180](https://github.com/apache/incubator-superset/pull/9180).
+
## Testing
### Python Testing
@@ -584,36 +657,9 @@ Note that the test environment uses a temporary directory
for defining the
SQLite databases which will be cleared each time before the group of test
commands are invoked.
-#### Typing
-
-To ensure clarity, consistency, all readability, _all_ new functions should use
-[type hints](https://docs.python.org/3/library/typing.html) and include a
-docstring using Sphinx documentation.
-
-Note per [PEP-484](https://www.python.org/dev/peps/pep-0484/#exceptions) no
-syntax for listing explicitly raised exceptions is proposed and thus the
-recommendation is to put this information in a docstring, i.e.,
-
-```python
-import math
-from typing import Union
-
-
-def sqrt(x: Union[float, int]) -> Union[float, int]:
- """
- Return the square root of x.
-
- :param x: A number
- :returns: The square root of the given number
- :raises ValueError: If the number is negative
- """
-
- return math.sqrt(x)
-```
-
-### JavaScript Testing
+### Frontend Testing
-We use [Jest](https://jestjs.io/) and [Enzyme](https://airbnb.io/enzyme/) to
test Javascript. Tests can be run with:
+We use [Jest](https://jestjs.io/) and [Enzyme](https://airbnb.io/enzyme/) to
test TypeScript/JavaScript. Tests can be run with:
```bash
cd superset-frontend
@@ -672,7 +718,7 @@ At runtime, the `_` function will return the translation of
the given
string for the current language, or the given string itself
if no translation is available.
-In JavaScript, the technique is similar:
+In TypeScript/JavaScript, the technique is similar:
we import `t` (simple translation), `tn` (translation containing a number).
```javascript
@@ -956,7 +1002,7 @@ Note not all fields are correctly catagorized. The fields
vary based on visualiz
| `row_limit`
| _number_ |
The **Row limit** widget |
| `timeseries_limit_metric`
| _object_ |
The **Sort By** widget |
-The `metric` (or equivalent) and `timeseries_limit_metric` fields are all
composed of either metric names or the JSON representation of the `AdhocMetric`
JavaScript type. The `adhoc_filters` is composed of the JSON represent of the
`AdhocFilter` JavaScript type (which can comprise of columns or metrics
depending on whether it is a WHERE or HAVING clause). The `all_columns`,
`all_columns_x`, `columns`, `groupby`, and `order_by_cols` fields all represent
column names.
+The `metric` (or equivalent) and `timeseries_limit_metric` fields are all
composed of either metric names or the JSON representation of the `AdhocMetric`
TypeScript type. The `adhoc_filters` is composed of the JSON represent of the
`AdhocFilter` TypeScript type (which can comprise of columns or metrics
depending on whether it is a WHERE or HAVING clause). The `all_columns`,
`all_columns_x`, `columns`, `groupby`, and `order_by_cols` fields all represent
column names.
### Filters Configuration
