-
Notifications
You must be signed in to change notification settings - Fork 1
Feat: Add Gerrit support to merge command #168
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
Feat: Add Gerrit support to merge command #168
Conversation
Add .gitignore and REUSE exemptions for local secrets files, used to test the new Gerrit functionality, since Gerrit API access needs credentials. Add pygerrit2>=2.0.15 to pyproject.toml and update uv.lock file. Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Add implementation plan for new Gerrit functionality. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Add new gerrit package and URL parser module implementing core Gerrit integration functionality for dependamerge. New gerrit/ package includes: - client.py: REST client with retry/timeout handling, XSSI guard stripping, and support for pygerrit2 or urllib fallback - urls.py: URL builder with automatic base path discovery - models.py: Pydantic models (GerritChangeInfo, GerritFileChange, GerritComparisonResult, etc.) paralleling GitHub PR models - service.py: High-level service layer for querying changes, finding similar changes, and managing pagination - comparator.py: Change comparison logic with automation detection for dependabot, pre-commit-ci, and other bot patterns - submit_manager.py: Parallel approval (+2 Code-Review) and submit operations with status tracking New url_parser.py module: - Unified URL parser distinguishing GitHub PRs from Gerrit changes - ChangeSource enum (GITHUB, GERRIT) for platform detection - ParsedUrl dataclass with platform-specific component extraction - Detection heuristics based on URL patterns (/pull/ vs /c/.../+/) These modules implement Phases 1-6 of the Gerrit support plan as documented in docs/GERRIT_IMPLEMENTATION.md. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Add comprehensive test suites covering all new Gerrit integration modules and URL parser functionality. Test files added: - test_gerrit_client.py: REST client tests including initialization, XSSI guard stripping, retry behavior, and authentication - test_gerrit_models.py: Pydantic model tests for GerritChangeInfo, GerritFileChange, GerritComparisonResult, and API response parsing - test_gerrit_service.py: Service layer tests for change fetching, open changes enumeration, and pagination handling - test_gerrit_comparator.py: Change comparison tests including subject matching, file comparison, and automation detection - test_gerrit_submit_manager.py: Submit manager tests for parallel approval and submission operations with status tracking - test_url_parser.py: URL parser tests for GitHub and Gerrit URL detection, parsing, and edge case handling These tests provide coverage for Phases 1-6 and Phase 8 of the Gerrit support plan as documented in docs/GERRIT_IMPLEMENTATION.md. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Update README.md to document the new Gerrit Code Review integration alongside the existing GitHub functionality. Changes include: Introduction and Overview: - Update tool description to mention Gerrit support - Expand merge command description with GitHub and Gerrit modes - Add Gerrit URL format examples - Update overview to list four main functions (adding Gerrit merge) Authentication section: - Add new Gerrit Authentication subsection - Document GERRIT_USERNAME and GERRIT_PASSWORD environment variables - Document fallback variables GERRIT_HTTP_USER and GERRIT_HTTP_PASSWORD - Add instructions for obtaining Gerrit HTTP credentials - Add Gerrit credential verification example Usage section: - Update Merging Pull Requests to show both GitHub and Gerrit examples - Explain automatic platform detection from URL format Command Options: - Reorganize merge options into General and GitHub-Specific sections - Add Gerrit Environment Variables subsection Examples section: - Add Example: Gerrit Merge with three usage scenarios - Show credential setup, custom threshold, and non-interactive modes Troubleshooting section: - Add Gerrit Authentication Error troubleshooting - Add Gerrit API Error troubleshooting - Add Gerrit URL Not Recognized troubleshooting Security Considerations: - Split into GitHub and Gerrit subsections - Add Gerrit-specific security guidance Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
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 pull request adds comprehensive Gerrit Code Review support to the dependamerge tool, enabling bulk review and submission of similar Gerrit changes alongside the existing GitHub PR functionality.
Changes:
- Adds URL parsing to detect and route between GitHub PRs and Gerrit changes
- Implements complete Gerrit integration including REST client, models, service layer, comparator, and submit manager
- Adds pygerrit2 dependency for Gerrit API interactions
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Added pygerrit2, pbr, and setuptools dependencies |
| pyproject.toml | Added pygerrit2>=2.0.15 dependency |
| src/dependamerge/url_parser.py | New module for detecting and parsing GitHub/Gerrit URLs |
| src/dependamerge/gerrit/init.py | Package exports for Gerrit modules |
| src/dependamerge/gerrit/client.py | REST client with retry logic and authentication |
| src/dependamerge/gerrit/models.py | Pydantic models for Gerrit changes and results |
| src/dependamerge/gerrit/service.py | High-level service for querying Gerrit |
| src/dependamerge/gerrit/comparator.py | Change comparison logic for similarity matching |
| src/dependamerge/gerrit/submit_manager.py | Parallel review and submit operations |
| src/dependamerge/gerrit/urls.py | URL construction with base path support |
| src/dependamerge/cli.py | Updated to route between GitHub and Gerrit handlers |
| tests/*.py | Comprehensive test coverage for all new modules |
| docs/GERRIT_IMPLEMENTATION.md | Implementation plan and documentation |
| README.md | Updated with Gerrit usage examples |
| REUSE.toml | Updated copyright exclusions |
| .gitignore | Updated secrets pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
Simplify Gerrit REST client by using pygerrit2 exclusively for all HTTP operations, removing the urllib fallback mechanism. Changes to client.py: - Remove urllib fallback code (~100 lines removed) - Remove PYGERRIT2_AVAILABLE conditional logic - Use pygerrit2.GerritRestAPI for GET, POST, PUT, DELETE methods - Add _extract_status_code() helper for requests exceptions - Fix retry logic to check transient errors in GerritRestError - Remove manual XSSI guard stripping (pygerrit2 handles this) - Remove base64/json/urllib imports no longer needed Changes to pyproject.toml: - Remove explicit requests dependency (comes via pygerrit2) Changes to __init__.py: - Remove PYGERRIT2_AVAILABLE from public exports Changes to tests: - Update all tests to mock pygerrit2.GerritRestAPI directly - Remove urllib-specific test mocks and monkeypatches - Add tests for PUT, DELETE, connection errors, timeouts - Add test for retry on transient network errors Benefits: - Single HTTP code path instead of dual urllib/pygerrit2 - Cleaner code (~95 lines removed from client.py) - pygerrit2 handles Gerrit quirks (XSSI guards, /a/ prefix) - Simpler tests without PYGERRIT2_AVAILABLE mocking Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
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
Copilot reviewed 22 out of 24 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Tested: |
e3dd042 to
77da305
Compare
77da305 to
2573840
Compare
2ada02a to
9d19ff2
Compare
Fixed with an additional commit.
Add support for loading Gerrit credentials from .netrc files, providing a more secure alternative to environment variables. Changes: - Add netrc.py module with NetrcParser and credential resolution - Add GerritCredentials dataclass for unified credential handling - Add resolve_gerrit_credentials() for centralized resolution - Add CLI options: --no-netrc, --netrc-file, --netrc-optional - Update build_client() to support netrc credential lookup - Display auth method in Gerrit change info table - Add comprehensive test coverage (51 netrc tests + 6 integration) Credential resolution priority: 1. CLI arguments (--username/--password) 2. .netrc file (searched in ./netrc, ~/.netrc, ~/_netrc) 3. Environment variables (GERRIT_USERNAME/GERRIT_PASSWORD) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
9d19ff2 to
38e0012
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
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The dependamerge "merge" sub-command can now be fed a Gerrit change URL. URLs are parsed for GitHub/Gerrit semantics and routed to the appropriate internal handler. Open changes in the Gerrit server are then scanned and scored for similarity, using the same heuristics as Github changes. Gerrit changes with merge conflicts are detected and reported, and they are rebased locally in order to highlight and report the nature of the conflict. More advanced behaviours and issue reporting/resolution during merging will require real world testing. Authentication to Gerrit will search common locations for a
.netrcfile, and will use those if parsing the file found a match for the target Gerrit server.Note: the code in this pull request has been tested against open dependabot changes in the Linux Foundation Gerrit server and is functional.