-
Notifications
You must be signed in to change notification settings - Fork 16
feat: match the rules with compile command in instrument phase #46
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
Conversation
Can I have a review? Thanks |
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 implements rule matching functionality in the instrumentation phase, allowing the tool to identify which rules apply to the package being compiled by the current build command. The changes refactor compile command detection and rule matching logic to be more modular and reusable across the codebase.
Key changes:
- Moved compile command utilities from setup package to util package for better reusability
- Implemented rule matching logic that compares compile command package paths with instrumentation rules
- Enhanced logging configuration to remove redundant time/level information and add phase context
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tool/util/go.go | New file containing extracted compile command detection and parsing utilities |
tool/util/util_test.go | Updated test file with correct package name and function references |
tool/internal/setup/find.go | Refactored to use moved utility functions from util package |
tool/internal/instrument/toolexec.go | Enhanced to check compile commands and match rules before instrumentation |
tool/internal/instrument/match.go | Implemented rule matching logic comparing package import paths |
tool/internal/instrument/load.go | Changed logging level from Info to Debug for rule loading |
tool/internal/ast/ast_test.go | New test file with proper package structure |
tool/cmd/main.go | Simplified logger configuration removing time formatting and adding phase context |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if err != nil { | ||
return err | ||
} | ||
// return nil |
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.
This commented-out return statement should be removed as it creates confusion about the intended control flow.
// return nil |
Copilot uses AI. Check for mistakes.
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.
This line is commented out because it is not implemented yet.
Subtask of #29
This patch checks which rules match the package being compiled by the current build command. In the next step, we will perform the actual instrumentation!