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

update git usage doc #1890

Merged
merged 3 commits into from
Apr 27, 2017
Merged

update git usage doc #1890

merged 3 commits into from
Apr 27, 2017

Conversation

livc
Copy link
Member

@livc livc commented Apr 25, 2017

solve #1889

提交你的代码时,pre-commit 钩子会检查本地代码是否存在
不适合提交的东西,等等。
```bash
➜ git status
Copy link
Collaborator

Choose a reason for hiding this comment

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

不是特别清楚这个是什么。

似乎不应该放到这个bash的coding block中。

Copy link
Member Author

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以上。
Copy link
Collaborator

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之上。

Copy link
Member Author

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` 完成。
Copy link
Collaborator

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个估计很难保证?我的每个commit里的修改往往都很小。整个PR的工作大概可以对应上述这个列表。

Copy link
Member Author

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

把这里的--rebase删除了吧,也没啥用。

Copy link
Member Author

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。
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个不是每一次都会出现的。。这个提示只会提示最近提交的commit。

我觉得如何建立PR的方法,我们应该引用github本身的文档,不应该自己写一遍。

Copy link
Member Author

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)
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
Member Author

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)
Copy link
Collaborator

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

Copy link
Member Author

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/) 的样式。
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doxygen 不定义代码样式,只是定义注释的格式吗?这句话应该是:

代码注释请遵守 Doxygen 格式。

Copy link
Member Author

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 已打开,并且编译能通过代码样式检查。
Copy link
Collaborator

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

Copy link
Member Author

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 已打开,并且编译能通过代码样式检查。
- 所有代码必须具有单元测试。
Copy link
Collaborator

Choose a reason for hiding this comment

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

读者会问单元测试怎么写?我们的单元测试是基于gtest framework的吗?

Copy link
Collaborator

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 到本地。
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
Member Author

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)。
Copy link
Collaborator

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。

Copy link
Member Author

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
Copy link
Collaborator

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是小写。

Copy link
Member Author

Choose a reason for hiding this comment

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

done

```

然后你可以通过做一个本地开发分支开始开发
也可以通过 `git checkout -b` 一次性创建并切换分支。
Copy link
Collaborator

Choose a reason for hiding this comment

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

我觉得不需要这句说明;可以直接在上面例子里使用 git checkout -b

Copy link
Member Author

Choose a reason for hiding this comment

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

done


## 保持 Fork 状态最新
## 提交(commit)
Copy link
Collaborator

@wangkuiyi wangkuiyi Apr 25, 2017

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"

关于构建和测试的更多信息,请参见这篇文档

Copy link
Member Author

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` 完成。
Copy link
Collaborator

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 时自动检查代码是否符合规范,并检查一些基本事宜,因此我们首先安装并在当前目录运行它。
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里需要说说安装 clang-format 吗?

Copy link
Collaborator

@reyoung reyoung left a 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.

@livc livc merged commit 73ccaeb into PaddlePaddle:develop Apr 27, 2017
@livc livc deleted the update_doc branch April 27, 2017 06:25
heavengate pushed a commit to heavengate/Paddle that referenced this pull request Aug 16, 2021
* support mask_rcnn for kunlun

* minor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants