-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add changelog command for archive comparison #92
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
base: main
Are you sure you want to change the base?
feat: Add changelog command for archive comparison #92
Conversation
This commit introduces a new `changelog` command to the `borg` CLI. This command generates a human-readable changelog between two archive versions, addressing a key user request for tracking changes over time. Key features of the `changelog` command include: - **File Comparison:** Detects added, modified, and removed files between two archives. - **Diff Statistics:** Calculates line-level insertions and deletions for modified files. - **Multiple Output Formats:** Supports plain text, Markdown, and JSON output, controlled by a `--format` flag. - **Remote Source Comparison:** Allows comparing a local archive against a remote GitHub repository using the `--source` flag (e.g., `--source github:org/repo`). - **Commit Message Extraction:** When comparing archives that contain a `.git` repository, the command extracts and displays the relevant commit messages for each modified file, providing valuable context for the changes. To support this functionality, this commit also includes: - A new `pkg/changelog` package containing the core logic for comparing archives and generating change reports. - A bugfix in `pkg/datanode` to ensure `fs.WalkDir` functions correctly on the root of a `DataNode`, which was necessary for iterating through archive contents. - A modification to the `pkg/vcs` Git cloner to include the `.git` directory in the created `DataNode`, enabling commit history analysis. Co-authored-by: Snider <631881+Snider@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Snider, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a powerful new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a new changelog command, which is a great addition. The implementation is well-structured and includes good test coverage. My review focuses on improving error handling and code structure in the core changelog generation logic to make it more robust and maintainable.
| exists, err := node.Exists(".git", datanode.ExistsOptions{WantType: fs.ModeDir}) | ||
| if err != nil || !exists { | ||
| return nil, os.ErrNotExist | ||
| } |
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.
The error handling here masks the original error from node.Exists. If node.Exists fails for a reason other than the file not existing (e.g., a permissions issue if this were a real filesystem), this logic will incorrectly report os.ErrNotExist. The original error should be checked first before checking for existence.
exists, err := node.Exists(".git", datanode.ExistsOptions{WantType: fs.ModeDir})
if err != nil {
return nil, err
}
if !exists {
return nil, os.ErrNotExist
}| commitMap, err := getCommitMap(newNode) | ||
| if err != nil { | ||
| // Not a git repo, so we can ignore this error | ||
| } |
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.
The error from getCommitMap is completely ignored. While it's acceptable to ignore os.ErrNotExist (when no .git directory is found), other errors (e.g., from a corrupted git repository) should be handled. Silently ignoring them can lead to an incomplete changelog without any indication of what went wrong.
| commitMap, err := getCommitMap(newNode) | |
| if err != nil { | |
| // Not a git repo, so we can ignore this error | |
| } | |
| commitMap, err := getCommitMap(newNode) | |
| if err != nil && !errors.Is(err, os.ErrNotExist) { | |
| return nil, fmt.Errorf("failed to process git history: %w", err) | |
| } |
| oldFiles := make(map[string][]byte) | ||
| if err := oldNode.Walk(".", func(path string, d fs.DirEntry, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if d.IsDir() || path == "." { | ||
| return nil | ||
| } | ||
| file, err := oldNode.Open(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer file.Close() | ||
| content, err := io.ReadAll(file) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| oldFiles[path] = content | ||
| return nil | ||
| }, walkOptions); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| newFiles := make(map[string][]byte) | ||
| if err := newNode.Walk(".", func(path string, d fs.DirEntry, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if d.IsDir() || path == "." { | ||
| return nil | ||
| } | ||
| file, err := newNode.Open(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer file.Close() | ||
| content, err := io.ReadAll(file) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| newFiles[path] = content | ||
| return nil | ||
| }, walkOptions); err != nil { | ||
| return nil, err | ||
| } |
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.
The logic to walk a DataNode and collect all file contents into a map is duplicated for oldNode and newNode. This can be extracted into a helper function to improve maintainability and reduce code duplication.
You could define a helper function like this:
func filesFromNode(node *datanode.DataNode, walkOptions datanode.WalkOptions) (map[string][]byte, error) {
files := make(map[string][]byte)
if err := node.Walk(".", func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() || path == "." {
return nil
}
file, err := node.Open(path)
if err != nil {
return err
}
defer file.Close()
content, err := io.ReadAll(file)
if err != nil {
return err
}
files[path] = content
return nil
}, walkOptions); err != nil {
return nil, err
}
return files, nil
}And then use it in GenerateReport to replace the duplicated blocks.
This change introduces a new
changelogcommand that can compare two archives, identify added/modified/removed files, calculate line-level diff statistics, and output the report in text, Markdown, or JSON formats. It also supports comparing a local archive with a remote GitHub repository and extracting commit messages from archives containing a.gitdirectory.Fixes #49
PR created automatically by Jules for task 5813737295717313175 started by @Snider