-
Notifications
You must be signed in to change notification settings - Fork 0
Version update #74
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
Version update #74
Conversation
BREAKING CHANGE: Use git commands directly instead of relying of go-git. This should allow for more compatibility with shallow clones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 400eb03 in 2 minutes and 39 seconds. Click for details.
- Reviewed
1667lines of code in28files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. branch_diff_commits.go:29
- Draft comment:
When NewHash fails (lines 29-32), the error is silently skipped. Consider logging the parsing error to help debug unexpected invalid hashes. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is suggesting a "consider logging" change, which is not a clear code change requirement. It's a suggestion for potential debugging improvements. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make speculative comments." This falls into the category of a speculative improvement - "consider logging to help debug unexpected invalid hashes" implies this might be useful IF there are unexpected invalid hashes, but there's no evidence this is actually a problem. The git command output should always be valid hashes, so this is a defensive suggestion rather than fixing an actual issue. The comment could be valid if there's a real risk of invalid hashes appearing in production and causing silent failures that would be hard to debug. Logging errors is generally a good practice, especially when silently skipping data. While logging is generally good practice, the comment uses "consider" language which makes it non-actionable. It's a suggestion rather than identifying a clear problem. The git command should always return valid hashes, so this is speculative about a scenario that shouldn't happen. This violates the rule about not making speculative comments. This comment should be deleted because it uses "consider" language making it a suggestion rather than a clear code change requirement, and it's speculative about debugging a scenario that shouldn't occur with valid git output.
2. get_tags.go:17
- Draft comment:
The custom delimiter '<TAG_SEP>' is used to split tag entries. Ensure that this delimiter cannot appear in tag data to avoid parsing errors. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that a custom delimiter does not appear in tag data, which is a form of asking for confirmation or verification. This violates the rule against asking the author to ensure behavior is intended or to double-check things. Therefore, this comment should not be approved.
3. previous_tag.go:29
- Draft comment:
The logic returns tags[0] when the current commit isn't tagged (lines 29-33). Confirm this behavior matches requirements, as it may be unexpected to return the most recent tag even if not attached to HEAD. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. branch_diff_commits.go:8
- Draft comment:
Typographical/grammatical suggestion: In the comment on line 8, it currently states 'returns of a diff of them.' Consider rephrasing to 'returns a diff of them' for clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely a typographical/grammatical suggestion, which is not related to the functionality or logic of the code. It doesn't provide a code suggestion or ask for a test, nor does it address any potential issues with the code itself. Therefore, it violates the rule against making purely informative comments.
5. commits_on_branch_simple.go:9
- Draft comment:
Typo in comment: Consider changing "slimed down commit object" to "slimmed down commit object". - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_9UcTTajr1J1QJM9w
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
|
||
| // Get commit message - use %B to get full body, then normalize | ||
| // go-git's Message field returns the raw commit message which always ends with \n | ||
| fullMessage, err := g.runGitCommand("log", "-1", "--format=%B", hashStr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple calls to g.runGitCommand (for commit message, author, committer, and dates) could be consolidated into a single command to improve performance.
| } | ||
|
|
||
| // Get message for this commit | ||
| message, err := g.runGitCommand("log", "-1", "--format=%B", hashLine) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each commit's message is fetched individually via a separate git log call (lines 42-46). Consider batching these calls if the branch has many commits to reduce overhead.
Important
Refactor git package to replace go-git with direct git command execution, updating commit, branch, and tag handling logic and tests.
BranchDiffCommitsinbranch_diff_commits.goto usegit logfor commit comparison, improving efficiency.Commitincommit.goto fetch commit details usinggit logcommands.CommitsBetweenincommits_between.goto handle empty hashes and usegit logfor commit retrieval.CommitsOnBranchandCommitsOnBranchSimpleincommits_on_branch.goandcommits_on_branch_simple.goto usegit logfor fetching commits.CurrentBranchandCurrentCommitincurrent_branch.goandcurrent_commit.goto usegitcommands for current state retrieval.CurrentTagincurrent_tag.goto determine the current tag usinggitcommands.getTagsinget_tags.goto fetch and sort tags usinggit for-each-ref.LatestCommitOnBranchinlatest_commit_on_branch.goto resolve branch commits usinggit rev-parse.PreviousTaginprevious_tag.goto find previous tags using sorted tag data.Hash,Commit,Signature, andReferencetypes intypes.goto replace go-git types.*_test.gofiles to align with new git command-based logic.test_helpers.gofor setting up test repositories and creating test data.go-gitand related dependencies fromgo.modandgo.sum.This description was created by
for 400eb03. You can customize this summary. It will automatically update as commits are pushed.