-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
update git usage doc #1890
update git usage doc #1890
Conversation
提交你的代码时,pre-commit 钩子会检查本地代码是否存在 | ||
不适合提交的东西,等等。 | ||
```bash | ||
➜ git status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不是特别清楚这个➜
是什么。
似乎不应该放到这个bash
的coding block中。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个可以用来区别输入的命令和命令的结果,比如
➜ git remote
origin
upstream
|
||
你可以通过 `pip install pre-commit` 安装 [pre-commit](http://pre-commit.com/), | ||
目前 Paddle 使用 `clang-format` 来调整C/C++源代码格式。请确保 clang-format 版本在3.8以上。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这一块对pre-commit的介绍最好加上,另外还有clang-format
的版本要在3.8之上。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
```shell | ||
git pull --rebase upstream develop | ||
Git 每次提交代码,都需要写提交说明,这可以让其他人知道这次提交做了哪些改变,这可以通过`git commit -m` 完成。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
永远禁止使用git commit -m
来写commit message。
使用git commit -m
倾向于将commit message写的含义不清,非常简短。正常的commit message分为如下几个部分。
80个字符之内介绍这个commit主要是做什么
[空行]
分条介绍这个commit的内容
例如
Add error clipping to MT demo.
* Compose GRU step naive layer in trainer config helpers.
* It is uses mixed_layer for gate.
* It supports ERROR_CLIPPING, DROPOUT
* Add error clipping in MT demo.
* Fix #1143
* Fix #1891
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个估计很难保证?我的每个commit里的修改往往都很小。整个PR的工作大概可以对应上述这个列表。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
```bash | ||
➜ git fetch upstream | ||
➜ git pull --rebase upstream develop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
把这里的--rebase
删除了吧,也没啥用。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
建立一个 Issue 描述问题,记录它的编号。 | ||
|
||
在 Push 新分支后, https://github.com/USERNAME/Paddle 中会出现新分支提示,点击绿色按钮发起 PR。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个不是每一次都会出现的。。这个提示只会提示最近提交的commit。
我觉得如何建立PR的方法,我们应该引用github本身的文档,不应该自己写一遍。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
在 Push 新分支后, https://github.com/USERNAME/Paddle 中会出现新分支提示,点击绿色按钮发起 PR。 | ||
|
||
![](https://ws1.sinaimg.cn/large/9cd77f2egy1fez1jq9mwdj21js04yq3m.jpg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
为啥要引用新浪的图片?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
在 PR 被 merge 进主仓库后,我们可以在 PR 的页面删除远程仓库的分支。 | ||
|
||
![](https://ws1.sinaimg.cn/large/9cd77f2egy1fez1pkqohzj217q05c0tk.jpg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
删除远程分支的命令是
git push origin :分支名
我自己一般做法是定期写一个shell清空这些东西。。不过我自己用fish shell,bash也是类似的语法。
for branch in (git branch --merged develop | grep -v develop | sed "s# ##g")
git branch -D $branch
git push origin :$branch
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -1,130 +1,185 @@ | |||
# 如何贡献代码 | |||
|
|||
我们真诚地感谢您的贡献,欢迎通过 GitHub 的 fork 和 pull request 流程来提交代码。 | |||
|
|||
## 代码要求 | |||
- 你的代码必须完全遵守 [doxygen](http://www.stack.nl/~dimitri/doxygen/) 的样式。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doxygen 不定义代码样式,只是定义注释的格式吗?这句话应该是:
代码注释请遵守 Doxygen 格式。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@@ -1,130 +1,185 @@ | |||
# 如何贡献代码 | |||
|
|||
我们真诚地感谢您的贡献,欢迎通过 GitHub 的 fork 和 pull request 流程来提交代码。 | |||
|
|||
## 代码要求 | |||
- 你的代码必须完全遵守 [doxygen](http://www.stack.nl/~dimitri/doxygen/) 的样式。 | |||
- 确保编译器选项 WITH\_STYLE\_CHECK 已打开,并且编译能通过代码样式检查。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WITH_STYLE_CHECK ==> WITH_STYLE_CHECK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@@ -1,130 +1,185 @@ | |||
# 如何贡献代码 | |||
|
|||
我们真诚地感谢您的贡献,欢迎通过 GitHub 的 fork 和 pull request 流程来提交代码。 | |||
|
|||
## 代码要求 | |||
- 你的代码必须完全遵守 [doxygen](http://www.stack.nl/~dimitri/doxygen/) 的样式。 | |||
- 确保编译器选项 WITH\_STYLE\_CHECK 已打开,并且编译能通过代码样式检查。 | |||
- 所有代码必须具有单元测试。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
读者会问单元测试怎么写?我们的单元测试是基于gtest framework的吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created an issue #1916 about this.
|
||
一旦你创建了一个fork,你可以使用你最喜欢的 git 客户端克隆你的仓库(repo)或只是直接在命令行输入: | ||
将远程仓库 clone 到本地。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
。 ==> :
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
## 创建本地分支 | ||
|
||
Paddle 目前使用[Git流分支模型](http://nvie.com/posts/a-successful-git-branching-model/)进行开发,测试,发行和维护。**develop** 是主分支,其他用户分支是特征分支(feature branches)。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PaddlePaddle 采用这个Git branching model。默认主分支是 develop,而不是master。
“其他分支”中包括master,而master并不是feature branch。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
```bash | ||
# (从当前分支)创建名为 MY_COOL_STUFF_BRANCH 的新分支 | ||
➜ git branch MY_COOL_STUFF_BRANCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MY_COOL_STUFF_BRANCH => my-cool-stuff
一般branch name是小写。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
``` | ||
|
||
然后你可以通过做一个本地开发分支开始开发 | ||
也可以通过 `git checkout -b` 一次性创建并切换分支。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我觉得不需要这句说明;可以直接在上面例子里使用 git checkout -b
。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
## 保持 Fork 状态最新 | ||
## 提交(commit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在这两个小结“开始开发”和“提交”之间得有一个小结介绍“构建和测试“。
这两个小结应该参考Build with Docker 文档。 刚刚请 @helinwang 发了一个 PR #1896 。 等merge之后,这里可以加一段:
构建和测试
编译 PaddlePaddle 的源码以及生成文档需要多种开发工具。为了方便大家,我们的标准开发流程是把这些工具都装进一个Docker image,称为开发镜像,通常名字是 paddle:dev
。然后所有用 cmake && make
的地方(比如IDE配置里)都用 docker run paddle:dev
来代替。
如要build这个开发镜像,在源码目录树的根目录中运行:
docker build -t paddle:dev .
随后可以用这个开发镜像开build PaddlePaddle的源码。比如如果要build一个不依赖GPU,但是支持AVX指令集,并且包括unit tests的PaddlePaddle,可以:
docker run -v $(pwd):/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=ON" paddle:dev
这个过程除了编译PaddlePaddle为 ./build/libpaddle.so
,并且输出一个 ./build/paddle.deb
文件之外,还会输出一个 build/Dockerfile
。我们只需要运行下面命令把编译好的PaddlePaddle打包成一个生产镜像(paddle:prod
):
docker build -t paddle:prod -f build/Dockerfile .
如果要运行所有的单元测试,可以用如下命令:
docker run -it -v $(pwd):/paddle paddle:dev bash -c "cd /paddle/build && ctest"
关于构建和测试的更多信息,请参见这篇文档。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
```shell | ||
git pull --rebase upstream develop | ||
Git 每次提交代码,都需要写提交说明,这可以让其他人知道这次提交做了哪些改变,这可以通过`git commit -m` 完成。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个估计很难保证?我的每个commit里的修改往往都很小。整个PR的工作大概可以对应上述这个列表。
``` | ||
|
||
用最新的 upstream 更新你的 fork: | ||
Paddle 使用 [pre-commit](http://pre-commit.com) 完成代码风格检查的自动化,它会在每次 commit 时自动检查代码是否符合规范,并检查一些基本事宜,因此我们首先安装并在当前目录运行它。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里需要说说安装 clang-format 吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but this commits need cherry-pick to release branch.
* support mask_rcnn for kunlun * minor
solve #1889