feat(commit-check): unify amend check rules into rebase and add git-flow instruction doc

docs/engineering-practice/git-flow-instructions_zh.md

@@ -0,0 +1,89 @@
+# 超实用! 从使用视角的Git协作实战，告别死记硬背
+
+## 一切从Fork开始
+* Git clone => 将远程仓库下载到本地
+* Git remote add  => 添加远程仓库地址
+
+## 开启一个新功能的开发
+* **推荐: 每个任务都对应独立的分支开发**
+* 从主分支切出你的开发分支
+    * Git checkout -b  => 切出开发分支
+    * Git fetch  =>  拉出远程分支
+    * Git rebase => 跟主分支保持一致
+
+## 如何创建【优雅的】commit？
+* 四步走
+    * Git diff => 先浏览一遍改动符不符合你预期
+    * Git status => 再看改动了哪些文件(这些文件的路径)
+    * Git add 文件 => 把文件添加到Git的暂存区
+    * Git commit => 真正开始提交
+
+* 什么样的commit是优雅的？
+    * **颗粒度和可读性非常重要**
+        * 颗粒度不能太大
+        * 一行文字能说清
+    * 为更好的利于别人理解，一般格式为 <type>(<scope>): <subject>
+        * 常见的type 分类:
+            * feat: 新功能、新特性
+            * fix: 修改 bug
+            * docs: 文档修改
+            * chore: 其他修改
+            * ci: 持续集成相关文件修改
+            * test: 测试用例新增、修改
+    * 参考阅读: [how to write good commit message](https://cbea.ms/git-commit/)
+
+## 如何创建【规范的】PR？
+* 推荐三步走
+    * Git fetch
+    * Git rebase
+    * Git push
+* **为什么推荐git fetch 和 git rebase呢?**
+    * 检查你的改动跟主分支是否冲突
+    * 让你的改动添加到主分支上，让合并后的主分支时间线更加的干净直观
+* 什么样的PR才是规范的？
+    * CI 检查必须要通过,除非失败是预期的
+    * PR Title 清晰，可理解
+    * 善用 PR 的Conversation区域，补充进一步的信息
+        * 必要时关联相关Issue
+* 要遵守什么样的Code Review礼仪？
+    * 保持心态: **giving and receiving**
+    * 参考阅读: [Kubernetes code review 规范](https://github.com/kubernetes/community/blob/master/contributors/guide/contributing.md#code-review)
+
+---
+> PS: 以下详细操作，请看DEMO
+---
+
+## 代码冲突了，怎么解决？
+* 先解决冲突
+* 再git add
+
+## 手滑了，产生了垃圾commit记录，怎么办？
+* Git rebase -i => 选择要整理的commit记录
+    * 记住: 已经合并到主分支的commits不要去动
+* Git push -f  => 如果已经推到远程仓库了，经过rebase整理后的commit，需要使用force push 才能重新推进去
+
+## 提交commit之后还想改动，但又不想产生新的commit记录，怎么办？
+* Git commit --amend
+
+## 能把代码提交到别人的仓库，集成之后再往主仓库提交吗？
+* 可以
+    * git remote add => 将对方的仓库加入Tracking列表
+    * git fetch => 拉取对方方库到本地
+    * git rebase => 将你的改动合并到目标分支的最前面
+    * git push => 提交到你自己的参考
+    * 针对对方仓库，正常创建PR
+
+## 当前功能还没做完，又需要紧急干另一个任务，该怎么办？
+* 可以直接正常提交PR，但 PR Tittle 带上[WIP] 标记
+* 当然也可以保存下当前的变动，转而切出新分支干另一个任务
+    * 基本步骤
+        * git stash
+        * git checkout -b <new_branch>
+        * git fetch
+        * git rebase 
+    * 如何恢复保存的临时变动？
+        * git stash apply 
+
+## 要针对线上版本做紧急Hotfix，该怎么办？
+* git checkout <commit>
+* git switch -c 

internal/linters/git-flow/commit-check/commit_check.go

@@ -71,6 +71,37 @@ func commitMessageCheckHandler(log *xlog.Logger, linterConfig config.Linter, age
 	return handle(context.Background(), log, agent, org, repo, author, number, toComments, existedComments)
 }
 
+func listCommits(ctx context.Context, agent linters.Agent, org, repo string, number int) ([]*github.RepositoryCommit, error) {
+	var preFilterCommits []*github.RepositoryCommit
+	opts := &github.ListOptions{}
+	commits, response, err := agent.GitHubClient().PullRequests.ListCommits(context.Background(), org, repo, number, opts)
+	if err != nil {
+		return preFilterCommits, err
+	}
+
+	if response.StatusCode != 200 {
+		log.Errorf("list commits failed: %v", response)
+		return preFilterCommits, fmt.Errorf("list commits failed: %v", response.Body)
+	}
+
+	return commits, nil
+}
+
+func listExistedComments(ctx context.Context, agent linters.Agent, org, repo string, number int) ([]*github.IssueComment, error) {
+	comments, resp, err := agent.GitHubClient().Issues.ListComments(ctx, org, repo, number, &github.IssueListCommentsOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	if resp.StatusCode != 200 {
+		log.Errorf("list comments failed: %v", resp)
+		return nil, fmt.Errorf("list comments failed: %v", resp.Body)
+	}
+
+	return comments, nil
+}
+
+// Deprecated: this is old version of commit check, which is not used anymore. Remove this after a while.
 const rebaseSuggestionFlag = "REBASE SUGGESTION"
 const commitCheckFlag = "[Git-flow]"
 
@@ -81,14 +112,9 @@ const commentTmpl = `**{{.Flag}}** Hi @{{.Author}}, There are some suggestions f
 {{range	.Comments}}
 > {{.}}
 {{end}}
+For other ` + "`" + `git-flow` + "`" + ` instructions, recommend refer to [these examples](https://github.com/qiniu/reviewbot/blob/master/docs/engineering-practice/git-flow-instructions_zh.md).
 
-<details>
-
-If you have any questions about this comment, feel free to raise an issue here:
-
-- **https://github.com/qiniu/reviewbot/issues**
-
-</details>
+{{.Footer}}
 `
 
 type RebaseSuggestion struct {
@@ -102,10 +128,12 @@ func handle(ctx context.Context, log *xlog.Logger, agent linters.Agent, org, rep
 		Flag     string
 		Author   string
 		Comments []string
+		Footer   string
 	}{
 		Flag:     commitCheckFlag,
 		Author:   author,
 		Comments: comments,
+		Footer:   linters.CommentFooter,
 	}
 
 	tmpl, err := template.New("").Parse(commentTmpl)
@@ -170,144 +198,92 @@ func handle(ctx context.Context, log *xlog.Logger, agent linters.Agent, org, rep
 	return nil
 }
 
-func listCommits(ctx context.Context, agent linters.Agent, org, repo string, number int) ([]*github.RepositoryCommit, error) {
-	var preFilterCommits []*github.RepositoryCommit
-	opts := &github.ListOptions{}
-	commits, response, err := agent.GitHubClient().PullRequests.ListCommits(context.Background(), org, repo, number, opts)
-	if err != nil {
-		return preFilterCommits, err
-	}
-
-	if response.StatusCode != 200 {
-		log.Errorf("list commits failed: %v", response)
-		return preFilterCommits, fmt.Errorf("list commits failed: %v", response.Body)
-	}
-
-	return commits, nil
-}
-
-func listExistedComments(ctx context.Context, agent linters.Agent, org, repo string, number int) ([]*github.IssueComment, error) {
-	comments, resp, err := agent.GitHubClient().Issues.ListComments(ctx, org, repo, number, &github.IssueListCommentsOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	if resp.StatusCode != 200 {
-		log.Errorf("list comments failed: %v", resp)
-		return nil, fmt.Errorf("list comments failed: %v", resp.Body)
-	}
-
-	return comments, nil
-}
-
 // Ruler is a function to check if commit messages match some rules
+// The message returned via Ruler will be added as part of the comment
+// so, It's recommended to use template rulerTmpl to generate a unified format message
 type Ruler func(log *xlog.Logger, commits []*github.RepositoryCommit) (string, error)
 
+const rulerTmpl = `
+### {{.Header}}
+{{.Message}}
+`
+
 var rulers = map[string]Ruler{
 	"Rebase suggestions": rebaseCheck,
-	"Amend suggestions":  amendCheck,
 }
 
 const (
-	pattern              = `^Merge (.*) into (.*)$`
-	rebaseSuggestionTmpl = `
-Your PR has **Merge** type commit messages like:
-{{range	.TargetCommits}}
-> {{.}}
-{{end}}
-Which seems insignificant, recommend to use` + ` **` + "`" + `git rebase <upstream>/<branch>` + "`" + `** command ` + `to reorganize your PR.
-`
+	pattern = `^Merge (.*) into (.*)$`
 )
 
 var mergeMsgRegex = regexp.MustCompile(pattern)
 
+// RebaseCheckRule checks if there are merge commit messages or duplicate messages in the PR
+// If there are, it will return a suggestion message to do git rebase
 func rebaseCheck(log *xlog.Logger, commits []*github.RepositoryCommit) (string, error) {
-	var preFilterCommits []string
+	var mergeTypeCommits []string
+	// filter out duplicated commit messages
+	var msgs = make(map[string]int, 0)
+
+	// filter out merge commit messages
 	for _, commit := range commits {
 		if commit.Commit != nil && commit.Commit.Message != nil {
 			if mergeMsgRegex.MatchString(*commit.Commit.Message) {
-				preFilterCommits = append(preFilterCommits, *commit.Commit.Message)
+				mergeTypeCommits = append(mergeTypeCommits, *commit.Commit.Message)
 			}
+			msgs[*commit.Commit.Message]++
 		}
 	}
 
-	// no matched commit messages
-	if len(preFilterCommits) == 0 {
-		return "", nil
-	}
-
-	var data = struct {
-		TargetCommits []string
-	}{
-		TargetCommits: preFilterCommits,
-	}
-
-	tmpl, err := template.New("").Parse(rebaseSuggestionTmpl)
-	if err != nil {
-		return "", err
-	}
+	var finalComments string
 
-	var buf bytes.Buffer
-	if err = tmpl.Execute(&buf, data); err != nil {
-		return "", err
-	}
-
-	suggestion := `
-### Rebase suggestions
-	` + buf.String()
-
-	return suggestion, nil
-}
-
-const (
-	amendSuggestionTmpl = `
-Your PR has **duplicate** commit messages like:
-{{range	.TargetCommits}}
-> {{.}}
-{{end}}
-Recommend to use` + ` **` + "`" + `git rebase -i ` + "`" + `** command ` + `to reorganize your PR.
-`
-)
-
-func amendCheck(log *xlog.Logger, commits []*github.RepositoryCommit) (string, error) {
-	var msgs = make(map[string]int, 0)
-	for _, commit := range commits {
-		if commit.Commit != nil && commit.Commit.Message != nil {
-			msgs[*commit.Commit.Message]++
+	// check if there are merge commit messages
+	if len(mergeTypeCommits) > 0 {
+		var cmtsForMergeTypeCommits string
+		for _, msg := range mergeTypeCommits {
+			cmtsForMergeTypeCommits += fmt.Sprintf("\n  > %s\n", msg)
 		}
+		finalComments += fmt.Sprintf("* Following commits seems generated via `git merge` \n %s", cmtsForMergeTypeCommits)
 	}
 
-	var preFilterCommits []string
+	// check if there are duplicated commit messages
+	var duplicatedCommits string
 	for msg, count := range msgs {
 		if count > 1 {
-			preFilterCommits = append(preFilterCommits, msg)
+			for i := 0; i < count; i++ {
+				duplicatedCommits += fmt.Sprintf("\n  > %s \n", msg)
+			}
 		}
 	}
 
-	if len(preFilterCommits) == 0 {
-		// no duplicated commit messages
-		return "", nil
+	if duplicatedCommits != "" {
+		finalComments += fmt.Sprintf("* Following commits have **duplicated** messages \n %s", duplicatedCommits)
 	}
 
-	var data = struct {
-		TargetCommits []string
-	}{
-		TargetCommits: preFilterCommits,
+	if finalComments == "" {
+		return "", nil
 	}
 
-	tmpl, err := template.New("").Parse(amendSuggestionTmpl)
+	// add footer
+	finalComments += "\n Which seems insignificant, recommend to use **`git rebase`** command to reorganize your PR. \n"
+
+	tmpl, err := template.New("ruler").Parse(rulerTmpl)
 	if err != nil {
 		return "", err
 	}
 
 	var buf bytes.Buffer
-	if err = tmpl.Execute(&buf, data); err != nil {
+	var data = struct {
+		Header  string
+		Message string
+	}{
+		Header:  "Rebase suggestions",
+		Message: finalComments,
+	}
+	err = tmpl.Execute(&buf, data)
+	if err != nil {
 		return "", err
 	}
 
-	suggestion := `
-### Amend suggestions
-	` + buf.String()
-
-	return suggestion, nil
+	return buf.String(), nil
 }