Skip to content

How to make a pull request

Jump to bottom
Yi Shen edited this page Sep 14, 2021 · 11 revisions

If you wish to make a pull request to the ECharts project, please read the following document.

Steps of Making a Pull Request

First-time Contributors

  1. Read the Apache Code of Conduct
  2. (Optional) If you don't know which issues to fix, read Finding Easy Issues to Fix
  3. Leave a comment on the issue and tell us that you are going to look into the issue
  4. (Optional) If you wish to discuss the issue, read Discussion
  5. Before you start writing code, read Coding Standard
  6. Read How to setup the dev environment

Making a Pull Request

  1. Name your branch for each issue
  2. Making changes
  3. (Optional) Add test cases
  4. Run test locally
  5. Commit your changes following Git Message Standard
  6. Push to remote and create a pull request
  7. Wait for the code review

Finding Easy Issues to Fix

If you don't have an idea about which issue to fix, you may use difficulty: easy label to filter issues that we think is easier to fix. These are issues that should be fixed using less time than the average. So it's a good way for beginners to make a PR.

You may also filter with en label for English issues only.

Discussion

If you wish to have a discussion about the issue you are fixing or get some help from the committers, please comment on issues or pull requests or discuss it with us in the mailing list.

Coding Standard

Please follow the coding standard before you make any changes.

It is recommended to install eslint plugin in your IDE to find the invalid code style. Otherwise, we can also use

npm run lint

to check the code style.

Name Your Branch

Fork ECharts project into your own project. Checkout a branch from master branch named fix-xxxx for each issue, where xxxx is the issue id related. If there's no related issue, you need to create one in most cases to describe what's wrong or what new feature is required.

git checkout master
git pull
git checkout -b fix-xxx

If you are a committer of the apache/echarts project, which means you have the write access to the project, you still need to push to a new branch (by git push origin HEAD:refs/heads/fix-xxxx) and use pull request to push your code. You cannot push code directly to master branch, otherwise, it will be rejected by GitHub.

Setup the dev environment

Checkout Wiki: How to setup the dev environment.

Test

In most cases, one or more test cases should be added when developing a feature or fixing a bug. All of the existing test cases are in directory ~/workspace/echarts/test. Check the file ~/workspace/echarts/test/dataZoom-action.html as an example.

Organize test cases

Each file can be regarded as a test suite and each chart in the file can be regarded as a test case, which contains one or multiple expected results (check points). If a feature or bug is related to a chart type or a component type, probably it should belong to a test file named chartOrComponentType-someSubCategory.html. Or some common feature is related to multiple charts or components or has nothing to do with chart and component, probably it should belong to a test file named featureName-someSubCateogory.html.

Naming of test files

Generally speaking, the name of the test file should start with a chart type or component type or a common feature name (like "hoverStyle", "clip").

Add a test case

If intending to add a test case, firstly try to find in the existing test files which file this new test case might belong to.

If an existing file found, add the test case to the file.

Otherwise, add a new test file by commands as follows:

# Make a file named "bar-action.html" in directory "echarts/test" with 1 initial chart.
npm run mktest bar-action
# or `npm run mktest bar-action.html`

# Make a file named "bar-action.html" in directory "echarts/test" with 5 initial charts.
npm run mktest bar-action 5

The expected results and the instructions of user interaction

Although we have an auto-visual-test tool to run tests, we should better write the expected result (check points) for each test case for manual checking.

Some cases need user interactions involved. The instructions should be written clearly.

The expected results and the user instructions should be written in the title filed when creating a test by testHelper.create as follows:

var chart = testHelper.create(echarts, 'main0', {
    title: [
        'Hover on the red circle',
        '**A blue label** should appear on the **top of red circle**.'
    ],
    option: option
});

Some notice of writing test cases

  1. Every test case should have a title to describe the feature to test.
  2. New test cases needs to been added at the bottom of each test file. To avoid the changed layout fail the recorded interactions.
  3. Test case should reflect the objectives of the test on the screenshot. For example, the animation duration needs to be long so the screenshot will be the intermediate state instead of the final state. The final state can't reflect the animation process.

Run test cases

Before making a pull request, you must run unit test locally:

npm run test

Visual tests are optional. If you changed view-related logic, please test the related visual cases. For example, if you add a new layout for pie chart labels, you should run the visual tests of all pie test cases and add new cases if needed.

Prepare local build:

# Visual test uses `dist/echarts.js` 
# To make a `dist/echarts.js`: 
npm run release
# (If using a symbol link zrender, we should `npm run prepublish` firstly in zrender).

Run visual test:

# Install puppeteer dependencies.
cd test/runTest
npm install
cd ../../
npm run test:visual

It will run all the test cases under ~/workspace/echarts/test automatically to compare with the previous version. You can use this to check if your code brings some breaking change.

Git Message Standard

Git commit messages are in the following format:

<type>(<scope>): <subject>. close #<issue_id>

In most cases, you need an issue to describe the pull request details and reference the issue number in the PR commit message. If not, the close #<issue_id> part could be omitted.

A typical git message looks like this:

feat(pie): provide two label layouts. close #1234

Types

<type> part must be one of the following:

  • build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
  • ci: Changes to our CI configuration files and scripts (example scopes: Circle, BrowserStack, SauceLabs)
  • docs: Documentation only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that neither fixes a bug nor adds a feature
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

Scopes

<scope> part could be:

  • pie/line/bar/...: Series names
  • visualMap/logAxis/legend/...: Component names
  • svg/ie/...: Environments
  • ...

Create a pull request

Fill in the templates when you create a pull request.

Clone this wiki locally