Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refine] refine contributing.md #1941

Merged
merged 38 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
0004627
refaictor CONTRIBUDING.MD
HAOCHENYE May 8, 2022
157be86
refaictor CONTRIBUDING.MD
HAOCHENYE May 8, 2022
c30bd98
fix source rvm shell command
HAOCHENYE May 8, 2022
b970b36
merge main
HAOCHENYE May 23, 2022
f649bc7
fix lint
HAOCHENYE May 23, 2022
6b18e6b
add chinese contributing.md
HAOCHENYE Jun 15, 2022
e6f14af
fix error render
HAOCHENYE Jun 15, 2022
d75b5dc
fix error render
HAOCHENYE Jun 15, 2022
a9357a8
refine description
HAOCHENYE Jun 15, 2022
ae650b9
refine description
HAOCHENYE Jun 15, 2022
615387b
minor refine
HAOCHENYE Jun 30, 2022
fc57e7c
install ffmpeg before ut
HAOCHENYE Jun 30, 2022
91d874b
Merge branch 'master' into HAOCHENYE/refine_contributing.md
HAOCHENYE Nov 13, 2022
04c7940
Minor retfine
HAOCHENYE Nov 13, 2022
1dff4d6
Minor refine
HAOCHENYE Nov 13, 2022
b28489b
Merge branch 'master' of github.com:open-mmlab/mmcv into HAOCHENYE/re…
HAOCHENYE Nov 16, 2022
f7b2184
refine chinese contributing.md
HAOCHENYE Nov 16, 2022
4596a1d
Refine
HAOCHENYE Nov 16, 2022
088f646
Fix as comment
HAOCHENYE Nov 16, 2022
c08f4f0
Fix as comment
HAOCHENYE Nov 16, 2022
25e0f39
restore pr.md
HAOCHENYE Nov 16, 2022
c6ede20
minor refine
HAOCHENYE Nov 16, 2022
c27ff8e
minor refine
HAOCHENYE Nov 16, 2022
b72abd8
update chinese CONTRIBUTING.md in root dir
HAOCHENYE Nov 16, 2022
70b7e00
adjust imgae size
HAOCHENYE Nov 17, 2022
23f5e26
update en CONTRIBUTING.md
HAOCHENYE Nov 17, 2022
6c78a8c
update en CONTRIBUTING.md
HAOCHENYE Nov 17, 2022
7991634
update contributing.md in docs
HAOCHENYE Nov 17, 2022
b0306be
restore change of index.rst
HAOCHENYE Nov 17, 2022
05b26cb
Fix as comment
HAOCHENYE Nov 17, 2022
0dfce37
minor refine
HAOCHENYE Nov 18, 2022
6a91ebd
remove content in PR.md
HAOCHENYE Nov 18, 2022
bb572c4
minor refine
HAOCHENYE Nov 18, 2022
353ee3b
minor refine
HAOCHENYE Nov 18, 2022
f830e56
minor refine
HAOCHENYE Nov 18, 2022
68fe606
Fix as comment
HAOCHENYE Nov 18, 2022
3016300
rename
HAOCHENYE Nov 18, 2022
dbd63c4
minor refine
HAOCHENYE Nov 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 222 additions & 23 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,214 @@
## Contributing to OpenMMLab

All kinds of contributions are welcome, including but not limited to the following.
Welcome to the MMCV community, we are committed to building a cutting-edge computer vision foundational library and all kinds of contributions are welcomed, including but not limited to

- Fix typo or bugs
- Add documentation or translate the documentation into other languages
- Add new features and components
**Fix bug**

### Workflow
You can directly post a Pull Request to fix typo in code or documents

1. fork and pull the latest OpenMMLab repository
2. checkout a new branch (do not use master branch for PRs)
3. commit your changes
4. create a PR
The steps to fix the bug of code implementation are as follows.

```{note}
If you plan to add some new features that involve large changes, it is encouraged to open an issue for discussion first.
1. If the modification involve significant changes, you should create an issue first and describe the error information and how to trigger the bug. Other developers will discuss with you and propose an proper solution.

2. Posting a pull request after fixing the bug and adding corresponding unit test.

**New Feature or Enhancement**

1. If the modification involve significant changes, you should create an issue to discuss with our developers to propose an proper design.
2. Post a Pull Request after implementing the new feature or enhancement and add corresponding unit test.

**Document**

You can directly post a pull request to fix documents. If you want to add a document, you should first create an issue to check if it is reasonable.

### Pull Request Workflow

If you're not familiar with Pull Request, don't worry! The following guidance will tell you how to create a Pull Request step by step. If you want to dive into the develop mode of Pull Request, you can refer to the [official documents](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests)

#### 1. Fork and clone

If you are posting a pull request for the first time, you should fork the OpenMMLab repositories by clicking the **Fork** button in the top right corner of the GitHub page, and the forked repositories will appear under your GitHub profile.

<img src="https://user-images.githubusercontent.com/57566630/167305749-43c7f4e9-449b-4e98-ade5-0c9276d5c9ce.png" width="1200">

Then, you can clone the repositories to local:

```shell
git clone git@github.com:{username}/mmcv.git
```

After that, you should ddd official repository as the upstream repository

```bash
git remote add upstream git@github.com:open-mmlab/mmcv
```

Check whether remote repository has been added successfully by `git remote -v`

```bash
origin git@github.com:{username}/mmcv.git (fetch)
origin git@github.com:{username}/mmcv.git (push)
upstream git@github.com:open-mmlab/mmcv (fetch)
upstream git@github.com:open-mmlab/mmcv (push)
```

> Here's a brief introduction to origin and upstream. When we use "git clone", we create an "origin" remote by default, which points to the repository cloned from. As for "upstream", we add it ourselves to point to the target repository. Of course, if you don't like the name "upstream", you could name it as you wish. Usually, we'll push the code to "origin". If the pushed code conflicts with the latest code in official("upstream"), we should pull the latest code from upstream to resolve the conflicts, and then push to "origin" again. The posted Pull Request will be updated automatically.

#### 2. Configure pre-commit

You should configure [pre-commit](https://pre-commit.com/#intro) in the local development environment to make sure the code style matches that of OpenMMLab. **Note**: The following code should be executed under the MMCV directory.

```shell
pip install -U pre-commit
pre-commit install
```

Check that pre-commit is configured successfully, and install the hooks defined in `.pre-commit-config.yaml`.

```shell
pre-commit run --all-files
```

<img src="https://user-images.githubusercontent.com/57566630/173660750-3df20a63-cb66-4d33-a986-1f643f1d8aaf.png" width="1200">

<img src="https://user-images.githubusercontent.com/57566630/202368856-0465a90d-8fce-4345-918e-67b8b9c82614.png" width="1200">

If the installation process is interrupted, you can repeatedly run `pre-commit run ... ` to continue the installation.

If the code does not conform to the code style specification, pre-commit will raise a warning and fixes some of the errors automatically.

<img src="https://user-images.githubusercontent.com/57566630/202369176-67642454-0025-4023-a095-263529107aa3.png" width="1200">

If we want to commit our code bypassing the pre-commit hook, we can use the `--no-verify` option(**only for temporarily commit**).

```shell
git commit -m "xxx" --no-verify
```

#### 3. Create a development branch

After configuring the pre-commit, we should create a branch based on the master branch to develop the new feature or fix the bug. The proposed branch name is `username/pr_name`

```shell
git checkout -b yhc/refactor_contributing_doc
```

In subsequent development, if the master branch of the local repository is behind the master branch of "upstream", we need to pull the upstream for synchronization, and then execute the above command:

```shell
git pull upstream master
```

#### 4. Commit the code and pass the unit test

- MMCV introduces mypy to do static type checking to increase the robustness of the code. Therefore, we need to add Type Hints to our code and pass the mypy check. If you are not familiar with Type Hints, you can refer to [this tutorial](https://docs.python.org/3/library/typing.html).

- The committed code should pass through the unit test

```shell
# Pass all unit tests
pytest tests

# Pass the unit test of runner
pytest tests/test_runner/test_runner.py
```

If the unit test fails for lack of dependencies, you can install the dependencies referring to the [guidance](#unit-test)

- If the documents are modified/added, we should check the rendering result referring to [guidance](#document-rendering)

#### 5. Push the code to remote

We could push the local commits to remote after passing through the check of unit test and pre-commit. You can associate the local branch with remote branch by adding `-u` option.

```shell
git push -u origin {branch_name}
```

This will allow you to use the `git push` command to push code directly next time, without having to specify a branch or the remote repository.

#### 6. Create a Pull Request

(1) Create a pull request in GitHub's Pull request interface

<img src="https://user-images.githubusercontent.com/57566630/201533288-516f7ac4-0b14-4dc8-afbd-912475c368b5.png" width="1200">

(2) Modify the PR description according to the guidelines so that other developers can better understand your changes

<img src="https://user-images.githubusercontent.com/57566630/202242953-c91a18ff-e388-4ff9-8591-5fae0ead6c1e.png" width="1200">

Find more details about Pull Request description in [pull request guidelines](#pr-specs).

**note**

(a) The Pull Request description should contain the reason for the change, the content of the change, and the impact of the change, and be associated with the relevant Issue (see [documentation](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)

(b) If it is your first contribution, please sign the CLA

<img src="https://user-images.githubusercontent.com/57566630/167307569-a794b967-6e28-4eac-a942-00deb657815f.png" width="1200">

(c) Check whether the Pull Request pass through the CI

<img src="https://user-images.githubusercontent.com/57566630/167307490-f9ebf9fa-63c0-4d83-8ba1-081ea169eb3a.png" width="1200">

MMCV will run unit test for the posted Pull Request on different platforms (Linux, Window, Mac), based on different versions of Python, PyTorch, CUDA to make sure the code is correct. We can see the specific test information by clicking `Details` in the above image so that we can modify the code.

(3) If the Pull Request passes the CI, then you can wait for the review from other developers. You'll modify the code based on the reviewer's comments, and repeat the steps [4](#4-commit-the-code-and-pass-the-unit-test)-[5](#5-push-the-code-to-remote) until all reviewers approve it. Then, we will merge it ASAP.

<img src="https://user-images.githubusercontent.com/57566630/202145400-cc2cd8c4-10b0-472f-ba37-07e6f50acc67.png" width="1200">

#### 7. Resolve conflicts

If your local branch conflicts with the latest master branch of "upstream", you'll need to resolove them. There are two ways to do this:

```shell
git fetch --all --prune
git rebase upstream/master
```

or

```shell
git fetch --all --prune
git merge upstream/master
```

If you are very good at handling conflicts, then you can use rebase to resolve conflicts, as this will keep your commit logs tidy. If you are not familiar with `rebase`, then you can use `merge` to resolve conflicts.

### Guidance

#### Unit test

If you cannot run the unit test of some modules for lacking of some dependencies, such as [video](https://github.com/open-mmlab/mmcv/tree/master/mmcv/video) module, you can try to install the following dependencies:

```shell
# Linux
sudo apt-get update -y
sudo apt-get install -y libturbojpeg
sudo apt-get install -y ffmpeg

# Windows
conda install ffmpeg
```

We should also make sure the committed code will not decrease the coverage of unit test, we could run the following command to check the coverage of unit test:

```shell
python -m coverage run -m pytest /path/to/test_file
python -m coverage html
# check file in htmlcov/index.html
```

#### Document rendering

If the documents are modified/added, we should check the rendering result. We could install the dependencies and run the following command to render the documents and check the results:

```shell
pip install -r requirements/docs.txt
cd docs/zh_cn/
# or docs/en
make html
# check file in ./docs/zh_cn/_build/html/index.html
```

### Code style
Expand All @@ -38,22 +232,27 @@ We use [pre-commit hook](https://pre-commit.com/) that checks and formats for `f
fixes `end-of-files`, `double-quoted-strings`, `python-encoding-pragma`, `mixed-line-ending`, sorts `requirments.txt` automatically on every commit.
The config for a pre-commit hook is stored in [.pre-commit-config](./.pre-commit-config.yaml).

After you clone the repository, you will need to install initialize pre-commit hook.
#### C++ and CUDA

```shell
pip install -U pre-commit
```
We follow the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html).

From the repository folder
### PR Specs

```shell
pre-commit install
```
1. Use [pre-commit](https://pre-commit.com) hook to avoid issues of code style

After this on every commit check code linters and formatter will be enforced.
2. One short-time branch should be matched with only one PR

> Before you create a PR, make sure that your code lints and is formatted by yapf.
3. Accomplish a detailed change in one PR. Avoid large PR

#### C++ and CUDA
- Bad: Support Faster R-CNN
- Acceptable: Add a box head to Faster R-CNN
- Good: Add a parameter to box head to support custom conv-layer number

We follow the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html).
4. Provide clear and significant commit message

5. Provide clear and meaningful PR description

- Task name should be clarified in title. The general format is: \[Prefix\] Short description of the PR (Suffix)
- Prefix: add new feature \[Feature\], fix bug \[Fix\], related to documents \[Docs\], in developing \[WIP\] (which will not be reviewed temporarily)
- Introduce main changes, results and influences on other modules in short description
- Associate related issues and pull requests with a milestone
Loading