Skip to content

Commit a49985f

Browse files
authored
[Docs] Add contributing.md and code_style.md (open-mmlab#754)
* add contributing.md * refine * Fix as comment
1 parent 5ea21f2 commit a49985f

File tree

7 files changed

+1549
-44
lines changed

7 files changed

+1549
-44
lines changed

CONTRIBUTING.md

+209-22
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,205 @@
11
## Contributing to OpenMMLab
22

3-
All kinds of contributions are welcome, including but not limited to the following.
3+
Welcome to the MMEngine 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
44

5-
- Fix typo or bugs
6-
- Add documentation or translate the documentation into other languages
7-
- Add new features and components
5+
**Fix bug**
86

9-
### Workflow
7+
You can directly post a Pull Request to fix typo in code or documents
108

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

16-
Note: If you plan to add some new features that involve large changes, it is encouraged to open an issue for discussion first.
11+
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.
1712

18-
### Code style
13+
2. Posting a pull request after fixing the bug and adding corresponding unit test.
1914

20-
#### Python
15+
**New Feature or Enhancement**
16+
17+
1. If the modification involve significant changes, you should create an issue to discuss with our developers to propose an proper design.
18+
2. Post a Pull Request after implementing the new feature or enhancement and add corresponding unit test.
19+
20+
**Document**
21+
22+
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.
23+
24+
### Pull Request Workflow
25+
26+
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)
27+
28+
#### 1. Fork and clone
29+
30+
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.
31+
32+
<img src="https://user-images.githubusercontent.com/57566630/167305749-43c7f4e9-449b-4e98-ade5-0c9276d5c9ce.png" width="1200">
33+
34+
Then, you can clone the repositories to local:
35+
36+
```shell
37+
git clone [email protected]:{username}/mmengine.git
38+
```
39+
40+
After that, you should ddd official repository as the upstream repository
41+
42+
```bash
43+
git remote add upstream [email protected]:open-mmlab/mmengine
44+
```
45+
46+
Check whether remote repository has been added successfully by `git remote -v`
47+
48+
```bash
49+
origin [email protected]:{username}/mmengine.git (fetch)
50+
origin [email protected]:{username}/mmengine.git (push)
51+
upstream [email protected]:open-mmlab/mmengine (fetch)
52+
upstream [email protected]:open-mmlab/mmengine (push)
53+
```
54+
55+
> 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.
56+
57+
#### 2. Configure pre-commit
58+
59+
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 mmengine directory.
60+
61+
```shell
62+
pip install -U pre-commit
63+
pre-commit install
64+
```
65+
66+
Check that pre-commit is configured successfully, and install the hooks defined in `.pre-commit-config.yaml`.
67+
68+
```shell
69+
pre-commit run --all-files
70+
```
71+
72+
<img src="https://user-images.githubusercontent.com/57566630/173660750-3df20a63-cb66-4d33-a986-1f643f1d8aaf.png" width="1200">
73+
74+
<img src="https://user-images.githubusercontent.com/57566630/202368856-0465a90d-8fce-4345-918e-67b8b9c82614.png" width="1200">
75+
76+
If the installation process is interrupted, you can repeatedly run `pre-commit run ... ` to continue the installation.
77+
78+
If the code does not conform to the code style specification, pre-commit will raise a warning and fixes some of the errors automatically.
79+
80+
<img src="https://user-images.githubusercontent.com/57566630/202369176-67642454-0025-4023-a095-263529107aa3.png" width="1200">
81+
82+
If we want to commit our code bypassing the pre-commit hook, we can use the `--no-verify` option(**only for temporarily commit**).
83+
84+
```shell
85+
git commit -m "xxx" --no-verify
86+
```
87+
88+
#### 3. Create a development branch
89+
90+
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`
91+
92+
```shell
93+
git checkout -b yhc/refactor_contributing_doc
94+
```
95+
96+
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:
97+
98+
```shell
99+
git pull upstream master
100+
```
101+
102+
#### 4. Commit the code and pass the unit test
103+
104+
- MMEngine 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).
105+
106+
- The committed code should pass through the unit test
107+
108+
```shell
109+
# Pass all unit tests
110+
pytest tests
111+
112+
# Pass the unit test of runner
113+
pytest tests/test_runner/test_runner.py
114+
```
115+
116+
If the unit test fails for lack of dependencies, you can install the dependencies referring to the [guidance](#unit-test)
117+
118+
- If the documents are modified/added, we should check the rendering result referring to [guidance](#document-rendering)
119+
120+
#### 5. Push the code to remote
121+
122+
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.
123+
124+
```shell
125+
git push -u origin {branch_name}
126+
```
127+
128+
This will allow you to use the `git push` command to push code directly next time without specifying a branch or the remote repository.
129+
130+
#### 6. Create a Pull Request
131+
132+
(1) Create a pull request in GitHub's Pull request interface
133+
134+
<img src="https://user-images.githubusercontent.com/57566630/201533288-516f7ac4-0b14-4dc8-afbd-912475c368b5.png" width="1200">
135+
136+
(2) Modify the PR description according to the guidelines so that other developers can better understand your changes
137+
138+
<img src="https://user-images.githubusercontent.com/57566630/202242953-c91a18ff-e388-4ff9-8591-5fae0ead6c1e.png" width="1200">
139+
140+
Find more details about Pull Request description in [pull request guidelines](#pr-specs).
141+
142+
**note**
143+
144+
(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)
145+
146+
(b) If it is your first contribution, please sign the CLA
147+
148+
<img src="https://user-images.githubusercontent.com/57566630/167307569-a794b967-6e28-4eac-a942-00deb657815f.png" width="1200">
149+
150+
(c) Check whether the Pull Request pass through the CI
151+
152+
<img src="https://user-images.githubusercontent.com/57566630/167307490-f9ebf9fa-63c0-4d83-8ba1-081ea169eb3a.png" width="1200">
153+
154+
MMEngine 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.
155+
156+
(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.
157+
158+
<img src="https://user-images.githubusercontent.com/57566630/202145400-cc2cd8c4-10b0-472f-ba37-07e6f50acc67.png" width="1200">
159+
160+
#### 7. Resolve conflicts
161+
162+
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:
163+
164+
```shell
165+
git fetch --all --prune
166+
git rebase upstream/master
167+
```
168+
169+
or
170+
171+
```shell
172+
git fetch --all --prune
173+
git merge upstream/master
174+
```
175+
176+
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.
177+
178+
### Guidance
179+
180+
#### Unit test
181+
182+
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:
183+
184+
```shell
185+
python -m coverage run -m pytest /path/to/test_file
186+
python -m coverage html
187+
# check file in htmlcov/index.html
188+
```
189+
190+
#### Document rendering
191+
192+
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:
193+
194+
```shell
195+
pip install -r requirements/docs.txt
196+
cd docs/zh_cn/
197+
# or docs/en
198+
make html
199+
# check file in ./docs/zh_cn/_build/html/index.html
200+
```
201+
202+
### Python Code style
21203

22204
We adopt [PEP8](https://www.python.org/dev/peps/pep-0008/) as the preferred code style.
23205

@@ -36,18 +218,23 @@ We use [pre-commit hook](https://pre-commit.com/) that checks and formats for `f
36218
fixes `end-of-files`, `double-quoted-strings`, `python-encoding-pragma`, `mixed-line-ending`, sorts `requirments.txt` automatically on every commit.
37219
The config for a pre-commit hook is stored in [.pre-commit-config](./.pre-commit-config.yaml).
38220

39-
After you clone the repository, you will need to install and initialize pre-commit hook.
221+
### PR Specs
40222

41-
```shell
42-
pip install -U pre-commit
43-
```
223+
1. Use [pre-commit](https://pre-commit.com) hook to avoid issues of code style
44224

45-
From the repository folder
225+
2. One short-time branch should be matched with only one PR
46226

47-
```shell
48-
pre-commit install
49-
```
227+
3. Accomplish a detailed change in one PR. Avoid large PR
228+
229+
- Bad: Support Faster R-CNN
230+
- Acceptable: Add a box head to Faster R-CNN
231+
- Good: Add a parameter to box head to support custom conv-layer number
232+
233+
4. Provide clear and significant commit message
50234

51-
After this on every commit check code linters and formatter will be enforced.
235+
5. Provide clear and meaningful PR description
52236

53-
> Before you create a PR, make sure that your code lints and is formatted by yapf.
237+
- Task name should be clarified in title. The general format is: \[Prefix\] Short description of the PR (Suffix)
238+
- Prefix: add new feature \[Feature\], fix bug \[Fix\], related to documents \[Docs\], in developing \[WIP\] (which will not be reviewed temporarily)
239+
- Introduce main changes, results and influences on other modules in short description
240+
- Associate related issues and pull requests with a milestone

0 commit comments

Comments
 (0)