-
Notifications
You must be signed in to change notification settings - Fork 0
fix: go module with replace #32
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
fix: go module with replace #32
Conversation
CodSpeed Performance ReportMerging #32 will not alter performanceComparing Summary
Benchmarks breakdown
|
613804f to
3a8a45b
Compare
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.
Pull Request Overview
This PR fixes support for Go modules with replace directives (fixes #31) and adds a dry-run mode for benchmarking. The key changes involve copying the entire git repository instead of just the module directory to properly support local replace directives, handling symlinks, and adding build-only mode for performance testing.
- Implements dry-run CLI flag that builds benchmarks without executing them
- Changes build process to copy entire git repository instead of just module directory to support replace directives
- Adds symlink handling in directory copying to follow directory symlinks
- Adds performance benchmarking infrastructure using divan/codspeed
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go-runner/src/cli.rs | Adds dry-run flag parsing and documentation |
| go-runner/src/lib.rs | Implements dry-run mode to skip benchmark execution and result collection |
| go-runner/src/utils.rs | Adds symlink handling and makes git repo path function public |
| go-runner/src/builder/templater.rs | Changes to copy git root instead of module dir and calculates module paths relative to git root |
| go-runner/src/builder/patcher.rs | Adds graceful error handling for unparseable Go files and optimization to skip parsing when unnecessary |
| go-runner/src/builder/discovery.rs | Optimizes benchmark detection by checking for "func Benchmark" before parsing |
| go-runner/tests/pkg_arg.rs | Updates tests to include dry_run field |
| go-runner/testdata/projects/example-with-replace/* | Adds new test project demonstrating replace directive support |
| go-runner/benches/go_runner.rs | Adds performance benchmark for the go-runner itself |
| go-runner/Cargo.toml | Adds divan benchmarking dependency and configuration |
| .github/workflows/codspeed.yml | Adds CI workflow for running benchmarks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9ad0d41 to
1f04f81
Compare
adriencaccia
left a comment
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.
lgtm, waiting on @GuillaumeLagrange's review
4342a96 to
efd03cb
Compare
GuillaumeLagrange
left a comment
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.
olgtm, next review should be almost instant
614fa45 to
d785401
Compare
GuillaumeLagrange
left a comment
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.
lgtm
d785401 to
1679125
Compare
Fixes #31