代码审查后更新拉取请求的首选 Github 工作流程

Posted

技术标签:

【中文标题】代码审查后更新拉取请求的首选 Github 工作流程【英文标题】:Preferred Github workflow for updating a pull request after code review 【发布时间】:2011-12-18 08:09:10 【问题描述】:

我已在 Github 上提交了对开源项目的更改,并收到了一位核心团队成员的代码审查 cmets。

我想在考虑到审查 cmets 的情况下更新代码,然后重新提交。执行此操作的最佳工作流程是什么?根据我对 git/github 的有限了解,我可以执行以下任何操作:

    将代码更新为新提交,并将初始提交和更新后的提交添加到我的拉取请求中。

    不知何故 (??) 从我的存储库中回滚旧提交,并创建一个包含所有内容的新提交,然后为此提出拉取请求?

    git commit 有一个修改功能,但我听说你在将提交推送到本地存储库之外后不应该使用它?在这种情况下,我在本地 PC 上进行了更改并推送到我的项目的 github 分支。使用“修正”可以吗?

    还有别的吗?

似乎选项 2/3 会很好,因为开源项目在他们的历史中只有一个提交可以实现所有内容,但我不知道该怎么做。

注意:我不知道这是否会影响答案,但我没有在单独的分支中进行更改,我只是在 master 之上进行了提交

【问题讨论】:

【参考方案1】:

更新拉取请求

要更新拉取请求(第 1 点),您唯一需要做的就是检查拉取请求所在的同一分支并再次推送到它:

cd /my/fork
git checkout master
...
git commit -va -m "Correcting for PR comments"
git push

可选 - 清理提交历史

您可能会被要求将您的提交压缩在一起,以便存储库历史记录是干净的,或者您自己想要删除中间提交,这些提交会分散您的拉取请求中的“消息”(第 2 点)。例如,如果您的提交历史如下所示:

$ git remote add parent git@github.com:other-user/project.git
$ git fetch parent
$ git log --oneline parent/master..master
e4e32b8 add test case as per PR comments
eccaa56 code standard fixes as per PR comments
fb30112 correct typos and fatal error
58ae094 fixing problem

将事物压缩在一起是一个好主意,以便它们显示为单个提交:

$ git rebase -i parent/master 

这将提示您选择如何重写您的拉取请求的历史记录,以下内容将在您的编辑器中:

pick 58ae094 fixing actual problem
pick fb30112 correct typos
pick eccaa56 code standard fixes
pick e4e32b8 add test case as per PR comments

对于您希望成为上一次提交的一部分的任何提交 - 将选择更改为 squash:

pick 58ae094 fixing actual problem
squash fb30112 correct typos
squash eccaa56 code standard fixes
squash e4e32b8 add test case as per PR comments

然后关闭你的编辑器。然后 Git 将重写历史并提示您为一个组合提交提供提交消息。进行相应的修改,您的提交历史现在将变得简洁:

$ git log --oneline parent/master..master
9de3202 fixing actual problem

把它推到你的叉子上:

$ git push -f
Counting objects: 19, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (11/11), 978 bytes, done.
Total 11 (delta 9), reused 7 (delta 6)
To git@github.com:me/my-fork.git
   f1238d0..9de3202  HEAD -> master

您的拉取请求将包含一个提交,将之前拆分为多个提交的所有更改合并。

改变公共回购的历史是一件坏事

重写历史记录并在可能已被其他人克隆的分支上使用git push -f 是一件坏事 - 它会导致存储库的历史记录和签出的历史记录发生分歧。

然而,修改你的 fork 的历史来更正你提议集成到存储库中的更改 - 是一件好事。因此,毫无保留地从您的拉取请求中消除“噪音”。

关于分支的注释

在上面,我将拉取请求显示为来自您 fork 的 master 分支,这没有什么问题,但它确实会产生某些限制,例如,如果这是您的标准技术,则只能每个存储库打开一个 PR。不过,为您希望提出的每个单独更改创建一个分支是一个更好的主意:

$ git branch feature/new-widgets
$ git checkout feature/new-widgets
...
Hack hack hack
...
$ git push
# Now create PR from feature/new-widgets

【讨论】:

+1 用于提及如何清理提交而不是推送额外的修复提交。 我在挑选/挤压时遇到了一些问题,this answer 帮助了我。还注意到在我做git push -f 之后,Github 删除了之前的对话。没有很多 cmets,但这是我没想到的。 要明确一点,当 rebase 有一个干净的历史时,你确实在改变你的公共提交,你只是假设没有人关心,因为它是一个 fork。 跟进:在 PR 期间更改 master 时的最佳实践? 请考虑重写已审查的拉取请求的历史记录(或通常评论/引用代码)可能会导致混乱,因为历史记录将不再匹配 cmets 所指的内容。没有简单的解决方案:有人会关闭 PR 并在新的 PR 中引用它(不重写历史);我的想法是仅备份正在重置/重写的最新提交 SHA,并在执行强制推送后在 PR 的评论中引用它。 如果 prune 没有删除那个分离的提交,那么它的历史仍然会匹配 PR 的 cmets。【参考方案2】:

只需将新提交添加到拉取请求中使用的分支,然后将分支推送到 GitHub。拉取请求将使用额外的提交自动更新。

#2 和#3 是不必要的。如果人们只想查看您的分支合并到的位置(而不是其他提交),他们可以使用git log --first-parent 仅查看日志中的合并提交。

【讨论】:

master 也是一个分支,所以从技术上讲它并不重要:) @OrionEdwards - 正如 poke 提到的,master 是一个分支,因此,更新它会导致基于它的任何拉取请求也被更新。 (这是为您计划提交拉取请求的任何内容使用单独分支的一个很好的理由。) 由于代码仍在 review 中,因此通常最好修复提交,而不是引入额外的修复提交,这只会使历史变得混乱...... @mgalgs 这是一个偏好问题。 由于the blog post I just wrote 中解释的原因,我不喜欢这个答案;我相信the other answer 会好很多。

以上是关于代码审查后更新拉取请求的首选 Github 工作流程的主要内容,如果未能解决你的问题,请参考以下文章

我们如何在 GitHub 中强制执行强制审查,但仍允许从 CI 发布 Maven?

git 从命令行 git pull 请求代码审查

VSTS拒绝了拉取请求完成工作流程

如何比较 Bitbucket 中的两个修订版?

我可以在 GitHub 上的 gist 上提出拉取请求吗?

让每个人都成为VSTS拉取请求的评论者