Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
33 changes: 33 additions & 0 deletions docs/dev_guides/code_review_cn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Code Review 约定

## 提交代码的一些约定
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不需要第三行的小标题了

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


为了使评审人在评审代码时更好地专注于代码本身,请您每次提交代码时,遵守以下约定:

1)请保证CI 中测试任务能顺利通过。如果没过,说明提交的代码存在问题,评审人一般不做评审。

2)提交PUll Request前:

- 请注意commit的数量:

原因:如果仅仅修改一个文件但提交了十几个commit,每个commit只做了少量的修改,这会给评审人带来很大困扰。评审人需要逐一查看每个commit才能知道做了哪些修改,且不排除commit之间的修改存在相互覆盖的情况。

建议:每次提交时,保持尽量少的commit,可以通过`git commit --amend`补充上次的commit。对已经Push到远程仓库的多个commit,可以参考[squash commits after push](http://stackoverflow.com/questions/5667884/how-to-squash-commits-in-git-after-they-have-been-pushed)。

- 请注意每个commit的名称:应能反映当前commit的内容,不能太随意。

3)如果解决了某个Issue的问题,请在该PUll Request的**第一个**评论框中加上:`fix #issue_number`,这样当该PUll Request被合并后,会自动关闭对应的Issue。关键词包括:close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved,请选择合适的词汇。详细可参考[Closing issues via commit messages](https://help.github.com/articles/closing-issues-via-commit-messages)。

此外,在回复评审人意见时,请您遵守以下约定:

1)评审人的每个意见都必须回复(这是开源社区的基本礼貌,别人帮了忙,应该说谢谢):

- 对评审意见同意且按其修改完的,给个简单的`Done`即可;

- 对评审意见不同意的,请给出您自己的反驳理由。

2)如果评审意见比较多:

- 请给出总体的修改情况。

- 请采用[start a review](https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/)进行回复,而非直接回复的方式。原因是每个回复都会发送一封邮件,会造成邮件灾难
33 changes: 33 additions & 0 deletions docs/dev_guides/code_review_en.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Code Reivew promise

## Certain regulations about submitting code

In order that reviewers focus on code in the code review,please follow these rules every time you submit your code:

1)Make sure that unit tests in CI pass through successfully.If it fails,it means problems have been found in submitted code which will not be reviewed by reviewer.

2)Before the submit of PUll Request:

- Please note the number of commit:

Reason:It will bother reviewers a lot if a dozen of commits are submitted after modification of only one file and only a few modifications are updated in every commit.Reviewers have to check commit one by one to figure out the modification.And sometimes it needs to take the overlap among commits into consideration.

Suggestion:Keep commit concise as much as possible at every submit.You can make a supplyment to the previous commit with `git commit --amend`.About several commits having been pushed to remote repository,you can refer to [squash commits after push](http://stackoverflow.com/questions/5667884/how-to-squash-commits-in-git-after-they-have-been-pushed)。

- Pay attention to the name of every commit:It would be better to abstract the content of present commit and be not too arbitrary.

3)If you have tackled with problems of an Issue,please add `fix #issue_number` to the *first* comment area of PULL Request.Then the corresponding Issue will be closed automatically after the merge of PULL Request.Keywords are including:close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved.Please select appropriate word.Please refer to [Closing issues via commit messages](https://help.github.com/articles/closing-issues-via-commit-messages) for more details.

In addition,please follow the following regulations in response to the suggestion of reviewers:

1)A reply to every comment of reviewers(It's a fundamental complimentary conduct in open source community.An expression of appreciation is a need for help from others):

- If you adopt the suggestion of reviewer and make a modification accordingly, it's courteous to reply with a simple `Done` .

- Please clarify your reason to the disagreenment

2)If there are many suggestions

- Please show general modification

- Please follow [start a review](https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/) to give your reply,instead of directly replying for that every comment will result in sending an email causing email disaster.
3 changes: 3 additions & 0 deletions docs/dev_guides/index_cn.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@
:hidden:

Overview_cn.md
local_dev_guide_cn.md
submit_pr_guide_cn.md
code_review_cn.md
18 changes: 18 additions & 0 deletions docs/dev_guides/index_en.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
########
贡献指南
########

我们非常欢迎你参与到飞桨的建设中,以下内容致力于说明所有加入飞桨的方式,并尽量帮助你顺利地贡献飞桨。

同样的,如果你觉得本篇文档有缺失,或者是有描述不清楚的地方,我们也非常欢迎你一同贡献本系列文档。

- `概述 <./Overview_cn.html>`_ : 贡献指南概述。


.. toctree::
:hidden:

Overview_en.md
local_dev_guide_en.md
submit_pr_guide_en.md
code_review_en.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

## 代码要求
- 代码注释请遵守 [Doxygen](http://www.doxygen.nl/) 的样式。
- 确保编译器选项 `WITH_STYLE_CHECK` 已打开,并且编译能通过代码样式检查。
- 所有代码必须具有单元测试。
- 通过所有单元测试。
- 请遵守[提交代码的一些约定](#提交代码的一些约定)。
- 请遵守[提交代码的一些约定](./code_review_cn.html)。

以下教程将指导您提交代码。
## [Fork](https://help.github.com/articles/fork-a-repo/)
Expand All @@ -26,7 +25,7 @@

## 创建本地分支

Paddle 目前使用[Git流分支模型](http://nvie.com/posts/a-successful-git-branching-model/)进行开发,测试,发行和维护,具体请参考 [Paddle 分支规范](https://github.com/PaddlePaddle/FluidDoc/blob/develop/doc/fluid/design/others/releasing_process.md)。
Paddle 目前使用[Git流分支模型](http://nvie.com/posts/a-successful-git-branching-model/)进行开发,测试,发行和维护,具体请参考 [Paddle 分支规范](https://github.com/PaddlePaddle/docs/blob/develop/docs/design/others/releasing_process.md)。

所有的 feature 和 bug fix 的开发工作都应该在一个新的分支上完成,一般从 `develop` 分支上创建新分支。

Expand All @@ -42,10 +41,10 @@ Paddle 目前使用[Git流分支模型](http://nvie.com/posts/a-successful-git-b

Paddle 开发人员使用 [pre-commit](http://pre-commit.com/) 工具来管理 Git 预提交钩子。 它可以帮助我们格式化源代码(C++,Python),在提交(commit)前自动检查一些基本事宜(如每个文件只有一个 EOL,Git 中不要添加大文件等)。

`pre-commit`测试是 Travis-CI 中单元测试的一部分,不满足钩子的 PR 不能被提交到 Paddle,首先安装并在当前目录运行它:
`pre-commit`测试是 CI 中单元测试的一部分,不满足钩子的 PR 不能被提交到 Paddle,Paddle使用的pre-commit是1.10.4版本。首先安装并在当前目录运行它:

```bash
➜ pip install pre-commit
➜ pip install pre-commit==1.10.4
➜ pre-commit install
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. clang-format要明确下是3.8版本,不是3.8以上
  2. 编译和单元测试,去掉单元测试。
  3. (TODO)编译要跳到 @pangyoki 新的链接里,可以等目录做完后,再次调整。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的,已经修改

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ You will learn how to develop programs in local environment under the guidelines

## Requirements of coding
- Please refer to the coding comment format of [Doxygen](http://www.doxygen.nl/)
- Make sure that option of builder `WITH_STYLE_CHECK` is on and the build could pass through the code style check.
- Unit test is needed for all codes.
- Pass through all unit tests.
- Please follow [regulations of submitting codes](#regulations of submitting codes).
- Please follow [regulations of submitting codes](./code_review_cn.html).

The following guidiance tells you how to submit code.
## [Fork](https://help.github.com/articles/fork-a-repo/)
Expand All @@ -26,7 +25,7 @@ Clone remote git to local:

## Create local branch

At present [Git stream branch model](http://nvie.com/posts/a-successful-git-branching-model/) is applied to Paddle to undergo task of development,test,release and maintenance.Please refer to [branch regulation of Paddle](https://github.com/PaddlePaddle/FluidDoc/blob/develop/doc/fluid/design/others/releasing_process.md) about details。
At present [Git stream branch model](http://nvie.com/posts/a-successful-git-branching-model/) is applied to Paddle to undergo task of development,test,release and maintenance.Please refer to [branch regulation of Paddle](https://github.com/PaddlePaddle/docs/blob/develop/docs/design/others/releasing_process.md) about details。

All development tasks of feature and bug fix should be finished in a new branch which is extended from `develop` branch.

Expand All @@ -44,11 +43,11 @@ It is worth noting that before the checkout, you need to keep the current branch

Paddle developers use the [pre-commit](http://pre-commit.com/) tool to manage Git pre-commit hooks. It helps us format the source code (C++, Python) and automatically check some basic things before committing (such as having only one EOL per file, not adding large files in Git, etc.).

The `pre-commit` test is part of the unit test in Travis-CI. A PR that does not satisfy the hook cannot be submitted to Paddle. Install `pre-commit` first and then run it in current directory:
The `pre-commit` test is part of the unit test in CI. A PR that does not satisfy the hook cannot be submitted to Paddle. The pre-commit used by Paddle is version 1.10.4. Install `pre-commit` first and then run it in current directory:


```bash
➜ pip install pre-commit
➜ pip install pre-commit==1.10.4
➜ pre-commit install
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# 提交PR注意事项

## 建立 Issue 并完成 Pull Request

建立一个 Issue 描述问题,并记录它的编号。
## 完成 Pull Request PR创建
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
这张图可以裁剪掉一些分支么,太大了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的


切换到所建分支,然后点击 `New pull request`。

Expand All @@ -16,6 +14,10 @@

接下来等待 review,如果有需要修改的地方,参照上述步骤更新 origin 中的对应分支即可。

### 关联Issue

如果解决了某个Issue的问题创建的PR,需要关联Issue,参考[Code Reivew 约定](./code_review_cn.html)
Copy link
Collaborator

@luotao1 luotao1 Mar 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19行可以合并到13行,都是关联issue,可以修改一下话术。去掉17-19行

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

话术合并后改成:
如果解决了某个Issue的问题创建的PR,需要关联Issue,参考Code Reivew 约定。并在 PR 的描述说明中,填写 resolve #Issue编号 可以在这个 PR 被 merge 后,自动关闭对应的 Issue,具体请见[这里]


## 签署CLA协议和通过单元测试

### 签署CLA
Expand Down Expand Up @@ -45,9 +47,7 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

46行的【提交commit】链接,链接过去的不对。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

请您关注您Pull Request中的CI单元测试进程,它将会在几个小时内完成
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO:等 @lelelelelez 的CI文章merge后,这里加一下链接


您仅需要关注和自己提交的分支相关的CI项目,例如您向develop分支提交代码,则无需关注release/1.1一栏是否通过测试

当所需的测试后都出现了绿色的对勾,表示您本次commit通过了各项单元测试
当所需的测试后都出现了绿色的对勾,表示您本次commit通过了各项单元测试,您只需要关注显示Required任务,不显示的可能是我们正在测试的任务

如果所需的测试后出现了红色叉号,代表您本次的commit未通过某项单元测试,在这种情况下,请您点击detail查看报错详情,并将报错原因截图,以评论的方式添加在您的Pull Request中,我们的工作人员将帮您查看

Expand Down Expand Up @@ -77,35 +77,3 @@
```

至此,我们就完成了一次代码贡献的过程。

## 提交代码的一些约定

为了使评审人在评审代码时更好地专注于代码本身,请您每次提交代码时,遵守以下约定:

1)请保证Travis-CI 中单元测试能顺利通过。如果没过,说明提交的代码存在问题,评审人一般不做评审。

2)提交PUll Request前:

- 请注意commit的数量:

原因:如果仅仅修改一个文件但提交了十几个commit,每个commit只做了少量的修改,这会给评审人带来很大困扰。评审人需要逐一查看每个commit才能知道做了哪些修改,且不排除commit之间的修改存在相互覆盖的情况。

建议:每次提交时,保持尽量少的commit,可以通过`git commit --amend`补充上次的commit。对已经Push到远程仓库的多个commit,可以参考[squash commits after push](http://stackoverflow.com/questions/5667884/how-to-squash-commits-in-git-after-they-have-been-pushed)。

- 请注意每个commit的名称:应能反映当前commit的内容,不能太随意。

3)如果解决了某个Issue的问题,请在该PUll Request的**第一个**评论框中加上:`fix #issue_number`,这样当该PUll Request被合并后,会自动关闭对应的Issue。关键词包括:close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved,请选择合适的词汇。详细可参考[Closing issues via commit messages](https://help.github.com/articles/closing-issues-via-commit-messages)。

此外,在回复评审人意见时,请您遵守以下约定:

1)评审人的每个意见都必须回复(这是开源社区的基本礼貌,别人帮了忙,应该说谢谢):

- 对评审意见同意且按其修改完的,给个简单的`Done`即可;

- 对评审意见不同意的,请给出您自己的反驳理由。

2)如果评审意见比较多:

- 请给出总体的修改情况。

- 请采用[start a review](https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/)进行回复,而非直接回复的方式。原因是每个回复都会发送一封邮件,会造成邮件灾难。
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Guide of submitting PR to Github

## Create an Issue and finish Pull Request
## Finish Pull Request create PR

Create an Issue to describe your problem and keep its number.

Expand All @@ -17,6 +17,10 @@ A note of `resolve #Issue number` in PR description results in automatic close o

Then please wait for review.If there is any need to make a modification,you can update corresponding branch in origin following the steps above.

### Link Issue

If a PR created by solving a problem of an Issue needs to be associated with the Issue, refer to [Code Reivew promise](./code_review_en.html)

## Sign CLA and pass unit tests

### Sign CLA
Expand Down Expand Up @@ -46,9 +50,7 @@ Every new commit in your Pull Request will trigger CI unit tests,so please make

Please note the procedure of CI unit tests in your Pull Request which will be finished in several hours.

You only need to focus on CI projects associated with your submitted branch.For example,there is no need to check whether release/1.1 pass test or not if you submit code to develop branch.

Green ticks after all tests means that your commit has passed all unit tests.
Green ticks after all tests means that your commit has passed all unit tests,you only need to focus on showing the Required tasks, the ones not showing may be the tasks we are testing.

Red cross after the tests means your commit hasn't passed certain unit test.Please click detail to view bug details and make a screenshot of bug,then add it as a comment in your Pull Request.Our stuff will help you check it.

Expand Down Expand Up @@ -77,35 +79,3 @@ Finally,we delete local branch
```

And now we finish a full process of code contribution

## Certain regulations about submitting code

In order that reviewers focus on code in the code review,please follow these rules every time you submit your code:

1)Make sure that unit tests in Travis-CI pass through successfully.If it fails,it means problems have been found in submitted code which will not be reviewed by reviewer.

2)Before the submit of PUll Request:

- Please note the number of commit:

Reason:It will bother reviewers a lot if a dozen of commits are submitted after modification of only one file and only a few modifications are updated in every commit.Reviewers have to check commit one by one to figure out the modification.And sometimes it needs to take the overlap among commits into consideration.

Suggestion:Keep commit concise as much as possible at every submit.You can make a supplyment to the previous commit with `git commit --amend`.About several commits having been pushed to remote repository,you can refer to [squash commits after push](http://stackoverflow.com/questions/5667884/how-to-squash-commits-in-git-after-they-have-been-pushed)。

- Pay attention to the name of every commit:It would be better to abstract the content of present commit and be not too arbitrary.

3)If you have tackled with problems of an Issue,please add `fix #issue_number` to the *first* comment area of PULL Request.Then the corresponding Issue will be closed automatically after the merge of PULL Request.Keywords are including:close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved.Please select appropriate word.Please refer to [Closing issues via commit messages](https://help.github.com/articles/closing-issues-via-commit-messages) for more details.

In addition,please follow the following regulations in response to the suggestion of reviewers:

1)A reply to every comment of reviewers(It's a fundamental complimentary conduct in open source community.An expression of appreciation is a need for help from others):

- If you adopt the suggestion of reviewer and make a modification accordingly, it's courteous to reply with a simple `Done` .

- Please clarify your reason to the disagreenment

2)If there are many suggestions

- Please show general modification

- Please follow [start a review](https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/) to give your reply,instead of directly replying for that every comment will result in sending an email causing email disaster.