-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add --include-internal flag, simplify CLI, and support non-invasive git analysis with filesystem comparison #2
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: latest
Are you sure you want to change the base?
Conversation
Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Remove the logic that skips internal packages entirely. The apidiff.Changes() function only analyzes exported (public) identifiers, so internal packages with public APIs should be included in the analysis. This ensures breaking changes to public APIs within internal packages are detected. Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
|
@copilot make a test cases to validate logic |
Added test suite to validate that: - Internal packages with public APIs are correctly analyzed - Breaking changes to exported identifiers are detected - Private (unexported) identifiers are ignored by apidiff - Compatible changes are properly identified - Various internal package path patterns are handled Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Added comprehensive test suite in commit 4562f0b. The tests validate:
All 7 test functions (11 total test cases) pass successfully. |
flaticols
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.
Well, I'm not sure that we cover all cases @copilot
|
Also, I think we should ignore internal packages by default, but be able to include them with the flag @copilot |
Extended test suite to cover additional scenarios: - Struct type changes (field additions/removals) - Interface changes (method additions) - Constant and variable changes - Method changes on types - New package additions (compatible) - Mixed compatible/incompatible changes across packages - Type alias changes Now covers 14 test functions with 18 sub-tests total, achieving 21.4% code coverage. Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Added 7 more comprehensive tests in commit 1105a2c to cover additional API change scenarios: New test coverage:
Total coverage now: 14 test functions with 18 sub-tests covering: All tests pass successfully. |
By default, internal packages are now excluded from API analysis as they are implementation details. Users can opt-in to include them using the --include-internal flag. This addresses the concern that internal packages should be skipped by default but optionally included. Changes: - Added --include-internal (-i) CLI flag - Modified OpenRepo and getPackages to accept includeInternal parameter - Restored conditional internal package filtering based on flag - Updated documentation to explain default behavior and flag usage - Added note in test file explaining the test coverage approach Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Implemented in commit d6f34c5. Internal packages are now excluded by default and can be included with the Default behavior: Skips internal packages (treating them as implementation details) The flag appears in the help output and allows users to opt-in when they need to track internal API stability. |
|
@copilot could we also change cli interface. I would like by default works with git like gobreaker oldcommit - works compare current commit with oldcommit or gobreaker oldcommit newcommit Or compare 2 different file system path. Ideas how to make it clear and easy to use? |
Changed from flag-based to positional argument interface: - gobreaker <old-ref> compares old-ref with HEAD - gobreaker <old-ref> <new-ref> compares two refs - Flags remain for optional settings (repo, format, include-internal, etc.) - More intuitive and easier to use interface Examples: gobreaker main # compare HEAD with main gobreaker main develop # compare two branches gobreaker abc123 def456 --repo /path # compare in different repo Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
|
And when we work with git gobreaker must current branch must not be touched by Copilot. |
Implemented in commit 63e8dfb. The CLI now uses positional arguments for a simpler, more intuitive interface: New usage:
Optional flags preserved:
The interface matches common git command patterns and requires less typing while remaining flexible with optional flags. |
No we can campare git and just fs packages without git repo as well |
The current implementation checks out commits to analyze them, which modifies the working tree. To avoid touching the current branch, I would need to either:
Which approach would you prefer? Also, should the tool still restore the original state after analysis (as it currently does with the defer on line 58-62)?
To support comparing filesystem packages without git, I can add a mode where:
Example usage:
Should I auto-detect based on whether the path exists on the filesystem, or would you prefer an explicit flag like |
Yes, must restore state. Also yes, let's auto detect that is path or any git ref. |
…ilesystem comparison mode Implemented two major improvements: 1. Git mode now uses temporary clones instead of checking out commits in the main worktree, ensuring the current branch is never modified 2. Added filesystem comparison mode to compare directories directly without requiring a git repository Features: - Auto-detects whether arguments are git refs or filesystem paths - Git mode: clones to temp dirs, checks out specific commits, cleans up after - Filesystem mode: directly compares Go packages in two directories - Both modes support --include-internal flag and all output formats Examples: gobreaker main develop # git mode gobreaker /path/old /path/new # filesystem mode gobreaker ./v1 ./v2 --include-internal # filesystem with internal pkgs Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Implemented in commit f6fc6b5. The tool now: Git mode (current branch untouched):
Filesystem mode (auto-detected):
Both modes support |
@copilot add more test cases |
Added 11 new test functions covering: - IsFilesystemPath: path detection logic with various scenarios - IsGitRef: git reference validation including invalid repos - getPackagesFromPath: loading packages from filesystem with internal package filtering - CompareFilesystems: filesystem comparison mode with breaking changes - Auto-detection: mixed mode validation - Complex scenarios: real-world multi-type changes Test improvements: - Coverage increased from 21.4% to 48.0% - Total test functions: 25 (was 14) - Total test cases: 42+ including sub-tests - All tests pass successfully New test coverage validates: - Filesystem comparison mode works correctly - Auto-detection properly distinguishes paths from git refs - Internal package filtering works in both git and filesystem modes - Error handling for invalid paths and missing repos - Complex API changes across multiple types simultaneously Co-authored-by: flaticols <7881854+flaticols@users.noreply.github.com>
Added 11 new comprehensive test functions in commit 7300be8: New test coverage:
Results:
The tests now comprehensively validate both git mode (non-invasive with temp clones) and filesystem comparison mode, including auto-detection, internal package handling, and error scenarios. |
By default, internal packages are now excluded from API analysis as they are implementation details not meant to be used outside the module. Users can opt-in to include them using the
--include-internalflag for tracking internal API stability.The CLI interface has been simplified to use positional arguments instead of required flags, making the tool more intuitive and easier to use.
The tool now supports two comparison modes with automatic detection: git mode (using temporary clones that never touch the current branch) and filesystem mode (comparing directories directly without requiring a git repository).
Changes
--include-internal(-i) CLI flag to optionally include internal packages in analysis/internal/in their path are excluded from analysisinternal/git/status.gobased on theincludeInternalparameterOpenRepo()and addedCompareFilesystems()functions for both comparison modesinternal/git/status_test.goto validate the logicTechnical Detail
Internal Package Filtering
The implementation now conditionally filters internal packages based on user preference:
When internal packages are included,
apidiff.Changes()analyzes only exported (public) identifiers, as it operates ontypes.Packagewhich inherently contains only exported APIs.CLI Interface
The CLI now uses positional arguments for a more intuitive interface:
New usage:
Before (flag-based):
After (positional arguments):
Optional flags (repo, format, include-internal, quiet, version) remain available for additional configuration.
Non-Invasive Git Analysis
Git mode now uses temporary clones to analyze commits without modifying the current working tree:
Filesystem Comparison Mode
Compare Go packages in any two directories without requiring a git repository:
Auto-Detection Logic
The tool automatically detects the comparison mode:
Usage
Git mode (compares commits without touching your current branch):
Filesystem mode (compares directories directly):
General examples:
Rationale
Internal Package Exclusion
Internal packages (with
/internal/in path) are Go's way of marking implementation details that cannot be imported from outside the module. Most users don't need to track changes to these packages since they're not part of the public API contract. However, large projects may want to maintain internal API stability, so the--include-internalflag provides that option.Simplified CLI
The positional argument approach matches common git command patterns, requires less typing, and is more intuitive while remaining flexible with optional flags.
Non-Invasive Analysis
Using temporary clones ensures that the tool never modifies your working tree or current branch, making it safe to use at any time without worrying about uncommitted changes or branch state.
Filesystem Mode
Supporting direct directory comparison enables use cases like comparing extracted release archives, validating build outputs, or analyzing packages without git history.
Testing
The test suite validates all types of API changes across various Go constructs:
Core functionality tests:
API change type coverage:
New feature test coverage:
Results:
The comprehensive test suite now validates both git mode (non-invasive with temporary clones) and filesystem comparison mode, including auto-detection, internal package handling, and error scenarios.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.