improved CreateBranch and CommitAndPushFiles performance
#163
Conversation
- Fix nil pointer dereference in CreateBranch method by adding proper null checks for sourceBranchRef and sourceBranchRef.Object - Fix TestGitHubClient_CreateBranch by using proper github.Reference mock instead of github.Branch - Fix TestGitHubClient_CommitAndPushFiles by adding second file to trigger multi-file commit path that matches test expectations
| Message: &commitMessage, | ||
| Content: []byte(encodedContent), | ||
| Branch: &branch, | ||
| Author: &github.CommitAuthor{ |
There was a problem hiding this comment.
so both for author and committer we are using author?
There was a problem hiding this comment.
In Git, there are two distinct roles for a commit:
- Author - The person who originally wrote the code/content
- Committer - The person who actually committed the code to the repository
While these are often the same person (as in your code example where both use the same values), they serve different purposes:
- The author represents who created the content
- The committer represents who added it to the repository
You need both because:
- Git's data model tracks these separately
- In collaborative workflows, they might be different people (e.g., someone submits code that another person reviews and commits)
- Setting both ensures proper attribution in the commit history
- It maintains consistency with standard Git behavior
Even when they're the same person, explicitly setting both fields ensures predictable behavior when interacting with GitHub's API.
vcsclient/github.go
Outdated
| Content: github.String(f.Content), | ||
| Encoding: github.String("utf-8"), | ||
| }) | ||
| if err != nil { |
There was a problem hiding this comment.
why are we continuing after we get an error?
i also see in the next loop that :
if result.err != nil {
close(blobChan)
return nil, fmt.Errorf("failed to create blob for %s: %w", result.file.Path, result.err)
}
so why it is not being stopped in the first iteration?
as concurrent creates race conditions
2 participants
go fmt ./...for formatting the code before submitting the pull request.Optimized file commits: Single files use Contents API, multiple files use Git API with parallel blob creation
Added concurrent processing for multi-file operations by using
createBlobsInParallel()Fixed nil pointer crash in
CreateBranchmethod by switching fromGetBranch()toGetRef()API and adding null checks