From 7f94ac3437fa5dced448f2c69a04cf058c51e155 Mon Sep 17 00:00:00 2001 From: HongyingG <44694098+HongyingG@users.noreply.github.com> Date: Mon, 7 Jan 2019 13:33:46 +0800 Subject: [PATCH] Submit pr guide (#521) * modify local_dev_guide * modification submit_pr_guide * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/local_dev_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update doc/fluid/advanced_usage/development/submit_pr_guide.md Co-Authored-By: HongyingG <44694098+HongyingG@users.noreply.github.com> * Update submit_pr_guide.md --- .../development/local_dev_guide.md | 193 ++++++++++++++++++ .../development/submit_pr_guide.md | 110 ++++++++++ 2 files changed, 303 insertions(+) create mode 100644 doc/fluid/advanced_usage/development/local_dev_guide.md create mode 100644 doc/fluid/advanced_usage/development/submit_pr_guide.md diff --git a/doc/fluid/advanced_usage/development/local_dev_guide.md b/doc/fluid/advanced_usage/development/local_dev_guide.md new file mode 100644 index 000000000..02e223f7d --- /dev/null +++ b/doc/fluid/advanced_usage/development/local_dev_guide.md @@ -0,0 +1,193 @@ +# Guide of local development + +You will learn how to develop programs in local environment under the guidelines of this document. + +## Requirements of coding +- Please refer to the coding comment format of [Doxygen](http://www.stack.nl/~dimitri/doxygen/) +- 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). + +The following guidiance tells you how to submit code. +## [Fork](https://help.github.com/articles/fork-a-repo/) + +Transfer to the home page of Github [PaddlePaddle](https://github.com/PaddlePaddle/Paddle) ,and then click button `Fork` to generate the git under your own file directory,such as 。 + +## Clone + +Clone remote git to local: + +```bash +➜ git clone https://github.com/USERNAME/Paddle +➜ cd Paddle +``` + + +## 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/Paddle/blob/develop/doc/design/releasing_process.md#paddle-分支规范) about details。 + +All development tasks of feature and bug fix should be finished in a new branch which is extended from `develop` branch. + +Create and switch to a new branch with command `git checkout -b`. + + +```bash +➜ git checkout -b my-cool-stuff +``` + +It is worth noting that before the checkout, you need to keep the current branch directory clean, otherwise the untracked file will be brought to the new branch, which can be viewed by `git status` . + + +## Use `pre-commit` hook + +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： + + +```bash +➜ pip install pre-commit +➜ pre-commit install +``` + +Paddle modify the format of C/C++ source code with `clang-format` .Make sure the version of `clang-format` is above 3.8. + +Note：There are differences between the installation of `yapf` with `pip install pre-commit` and that with `conda install -c conda-forge pre-commit` . Paddle developers use `pip install pre-commit` 。 + +## Start development + +I delete a line of README.md and create a new file in the case. + +View the current state via `git status` , which will prompt some changes to the current directory, and you can also view the file's specific changes via `git diff` . + + +```bash +➜ git status +On branch test +Changes not staged for commit: + (use "git add ..." to update what will be committed) + (use "git checkout -- ..." to discard changes in working directory) + modified: README.md +Untracked files: + (use "git add ..." to include in what will be committed) + test +no changes added to commit (use "git add" and/or "git commit -a") +``` + +## Build and test + +It needs a variety of development tools to build PaddlePaddle source code and generate documentation. For convenience, our standard development procedure is to put these tools together into a Docker image,called *development mirror* , usually named as `paddle:latest-dev` or `paddle:[version tag]-dev`,such as `paddle:0.11.0-dev` . Then all that need `cmake && make` ,such as IDE configuration,are replaced by `docker run paddle:latest-dev` . + +You need to bulid this development mirror under the root directory of source code directory tree + +```bash +➜ docker build -t paddle:latest-dev . +``` + +Then you can start building PaddlePaddle source code with this development mirror.For example,to build a Paddleddle which are not dependent on GPU but in support of AVX commands and including unit test,you can: + +```bash +➜ docker run -v $(pwd):/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TESTING=ON" paddle:latest-dev +``` + +If you want to build PaddlePaddle based on Python3,you can: + +```bash +➜ docker run -v $(pwd):/paddle -e "PY_VERSION=3.5" -e "WITH_FLUID_ONLY=ON" -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TESTING=ON" paddle:latest-dev +``` + +Except for the build of PaddlePaddle as `./build/libpaddle.so` and the output of `./build/paddle.deb` file, there is an output of `build/Dockerfile`. What we need to do is to package the PaddlePaddle as a *produce mirror*（ `paddle:prod` ）with following commands. + +```bash +➜ docker build -t paddle:prod -f build/Dockerfile . +``` + +Run all unit tests with following commands: + +```bash +➜ docker run -it -v $(pwd):/paddle paddle:latest-dev bash -c "cd /paddle/build && ctest" +``` + +Please refer to [Installation and run with Docker](https://github.com/PaddlePaddle/Paddle/blob/develop/doc/v2/build_and_install/docker_install_cn.rst) about more information of construction and test. + +## Commit + +Next we cancel the modification of README.md,and submit new added test file. + +```bash +➜ git checkout -- README.md +➜ git status +On branch test +Untracked files: + (use "git add ..." to include in what will be committed) + test +nothing added to commit but untracked files present (use "git add" to track) +➜ git add test +``` + +It's required that the commit message is also given on every Git commit, through which other developers will be notified of what changes have been made. Type `git commit` to realize it. + +```bash +➜ git commit +CRLF end-lines remover...............................(no files to check)Skipped +yapf.................................................(no files to check)Skipped +Check for added large files..............................................Passed +Check for merge conflicts................................................Passed +Check for broken symlinks................................................Passed +Detect Private Key...................................(no files to check)Skipped +Fix End of Files.....................................(no files to check)Skipped +clang-formater.......................................(no files to check)Skipped +[my-cool-stuff c703c041] add test file + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 233 +``` + + Attention needs to be paid：you need to add commit message to trigger CI test.The command is as follows: + +```bash +# Touch CI single test of develop branch +➜ git commit -m "test=develop" +# Touch CI single test of release/1.1 branch +➜ git commit -m "test=release/1.1" +``` + +## Keep the latest local repository + +It needs to keep up with the latest code of original repository(）before Pull Request. + +Check the name of current remote repository with `git remote`. + +```bash +➜ git remote +origin +➜ git remote -v +origin https://github.com/USERNAME/Paddle (fetch) +origin https://github.com/USERNAME/Paddle (push) +``` + +origin is the name of remote repository that we clone,which is also the Paddle under your own account. Next we create a remote host of an original Paddle and name it upstream. + +```bash +➜ git remote add upstream https://github.com/PaddlePaddle/Paddle +➜ git remote +origin +upstream +``` + +Get the latest code of upstream and update current branch. + +```bash +➜ git fetch upstream +➜ git pull upstream develop +``` + +## Push to remote repository + +Push local modification to GitHub(https://github.com/USERNAME/Paddle). + +```bash +# submit it to remote git the branch my-cool-stuff of origin +➜ git push origin my-cool-stuff +``` diff --git a/doc/fluid/advanced_usage/development/submit_pr_guide.md b/doc/fluid/advanced_usage/development/submit_pr_guide.md new file mode 100644 index 000000000..d71d92e36 --- /dev/null +++ b/doc/fluid/advanced_usage/development/submit_pr_guide.md @@ -0,0 +1,110 @@ +# Guide of submitting PR to Github + +## Create an Issue and finish Pull Request + +Create an Issue to describe your problem and keep its number. + +Switch to the branch you have created and click `New pull request`。 + +screen shot 2017-04-26 at 9 09 28 pm + +Switch to targeted branch: + +screen shot 2017-04-26 at 9 11 52 pm + +A note of `resolve #Issue number` in PR description results in automatic close of corresponding Issue after the merge of PR.More details can be viewed [here](https://help.github.com/articles/closing-issues-via-commit-messages/)。 + +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. + +## Sign CLA and pass unit tests + +### Sign CLA + +For the first time to submit Pull Request,you need to sign CLA(Contributor License Agreement) to ensure merge of your code.Specific steps are listed as follows: + +- Please check the Check in PR to find license/cla and click detail on the right to change into CLA website. + +
+ + + +
+ +- Please click “Sign in with GitHub to agree” in CLA website.It will change into your Pull Request page after the click. + +
+ + + +
+ + +### Pass unit tests + +Every new commit in your Pull Request will trigger CI unit tests,so please make sure that necessary comments have been included in your commit message.Please refer to [commit](local_dev_guide.html#permalink-8--commit-) + +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. + +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. + + +## Delete remote branch + +We can delete branches of remote repository in PR page after your PR is successfully merged into master repository. + +screen shot 2017-04-26 at 9 18 24 pm + +We can also delete the branch of remote repository with `git push origin :the_branch_name`,such as: + +```bash +➜ git push origin :my-cool-stuff +``` + +## Delete local branch + +Finally,we delete local branch + +```bash +# Switch to develop branch +➜ git checkout develop +# delete my-cool-stuff branch +➜ git branch -D my-cool-stuff +``` + +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. -- GitLab