[GitHub] [incubator-devlake] hezyin commented on a diff in pull request #2050: issues/1914: refactor run() method

[GitBox <gi...@apache.org>]

hezyin commented on code in PR #2050:
URL: https://github.com/apache/incubator-devlake/pull/2050#discussion_r889491887


##########
plugins/gitextractor/parser/libgit2.go:
##########
@@ -186,96 +187,126 @@ func (l *LibGit2) run(repo *git.Repository, repoId string) error {
 			c.CommitterId = committer.Email
 			c.CommittedDate = committer.When
 		}
-		var commitParents []*code.CommitParent
-		for i := uint(0); i < commit.ParentCount(); i++ {
-			parent := commit.Parent(i)
-			if parent != nil {
-				if parentId := parent.Id(); parentId != nil {
-					commitParents = append(commitParents, &code.CommitParent{
-						CommitSha:       c.Sha,
-						ParentCommitSha: parentId.String(),
-					})
-				}
-			}
-		}
-		err2 := l.store.CommitParents(commitParents)
-		if err2 != nil {
-			return err2
+		if err != l.storeParentCommits(commitSha, commit) {
+			return err
 		}
 		if commit.ParentCount() > 0 {
 			parent := commit.Parent(0)
 			if parent != nil {
-				var parentTree, tree *git.Tree
-				parentTree, err2 = parent.Tree()
-				if err2 != nil {
-					return err2
-				}
-				tree, err2 = commit.Tree()
-				if err2 != nil {
-					return err2
-				}
-				var diff *git.Diff
-				diff, err2 = repo.DiffTreeToTree(parentTree, tree, &opts)
-				if err2 != nil {
-					return err2
-				}
-				var commitFile *code.CommitFile
-				err2 = diff.ForEach(func(file git.DiffDelta, progress float64) (
-					git.DiffForEachHunkCallback, error) {
-					if commitFile != nil {
-						err2 = l.store.CommitFiles(commitFile)
-						if err2 != nil {
-							l.logger.Error("CommitFiles error:", err)
-							return nil, err2
-						}
-					}
-					commitFile = new(code.CommitFile)
-					commitFile.CommitSha = c.Sha
-					commitFile.FilePath = file.NewFile.Path
-					return func(hunk git.DiffHunk) (git.DiffForEachLineCallback, error) {
-						return func(line git.DiffLine) error {
-							if line.Origin == git.DiffLineAddition {
-								commitFile.Additions += line.NumLines
-							}
-							if line.Origin == git.DiffLineDeletion {
-								commitFile.Deletions += line.NumLines
-							}
-							return nil
-						}, nil
-					}, nil
-				}, git.DiffDetailLines)
-				if err2 != nil {
-					return err2
-				}
-				if commitFile != nil {
-					err2 = l.store.CommitFiles(commitFile)
-					if err2 != nil {
-						l.logger.Error("CommitFiles error:", err)
-					}
-				}
 				var stats *git.DiffStats
-				stats, err2 = diff.Stats()
-				if err2 != nil {
-					return err2
+				if stats, err = l.getDiffComparedToParent(c.Sha, commit, parent, repo, opts); err != nil {
+					return err
+				} else {
+					c.Additions += stats.Insertions()
+					c.Deletions += stats.Deletions()
 				}
-				c.Additions += stats.Insertions()
-				c.Deletions += stats.Deletions()
 			}
 		}
-		err2 = l.store.Commits(c)
-		if err2 != nil {
-			return err2
+		err = l.store.Commits(c)
+		if err != nil {
+			return err
 		}
 		repoCommit := &code.RepoCommit{
 			RepoId:    repoId,
 			CommitSha: c.Sha,
 		}
-		err2 = l.store.RepoCommits(repoCommit)
-		if err2 != nil {
-			return err2
+		err = l.store.RepoCommits(repoCommit)
+		if err != nil {
+			return err
 		}
 		l.subTaskCtx.IncProgress(1)
 		return nil
 	})
-	return err
+}
+
+func (l *LibGit2) storeParentCommits(commitSha string, commit *git.Commit) error {
+	var commitParents []*code.CommitParent
+	for i := uint(0); i < commit.ParentCount(); i++ {
+		parent := commit.Parent(i)
+		if parent != nil {
+			if parentId := parent.Id(); parentId != nil {
+				commitParents = append(commitParents, &code.CommitParent{
+					CommitSha:       commitSha,
+					ParentCommitSha: parentId.String(),
+				})
+			}
+		}
+	}
+	return l.store.CommitParents(commitParents)
+}
+
+func (l *LibGit2) getDiffComparedToParent(commitSha string, commit *git.Commit, parent *git.Commit, repo *git.Repository, opts *git.DiffOptions) (*git.DiffStats, error) {
+	var err error
+	var parentTree, tree *git.Tree
+	parentTree, err = parent.Tree()
+	if err != nil {
+		return nil, err
+	}
+	tree, err = commit.Tree()
+	if err != nil {
+		return nil, err
+	}
+	var diff *git.Diff
+	diff, err = repo.DiffTreeToTree(parentTree, tree, opts)
+	if err != nil {
+		return nil, err
+	}
+	var commitFile *code.CommitFile
+	commitFile, err = l.generateCommitFileFromDiff(commitSha, diff)

Review Comment:
   @keon94 This PR splits the original giant `run` function pretty well except this `generateCommitFileFromDiff` function. In fact, the code from line 256 to 264 should live together with the logic in `generateCommitFileFromDiff`. For each diff, we extract and store multiple `commitFile` into the database. Line 256 to 264 is just handling the last one.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org