Conversation
Design for extracting Carson::Remote, Carson::PullRequest, and Carson::Branch from the Runtime god object. Approved design with error contract (raise on failure), wiring pattern (thin Runtime delegates), and scoped extraction (lifecycle only, govern/status deferred to second pass). Tracks #246. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
16-task TDD plan covering Remote, PullRequest, Branch extraction and template sync regression test. Each task is bite-sized with exact file paths, test code, implementation code, and verification commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, 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 comprehensive documentation for a significant architectural refactoring within the Highlights
Changelog
Activity
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.
Code Review
This pull request introduces new linter configurations for Biome, RuboCop, and Ruff, enhancing code quality and consistency across different language ecosystems within the repository. Additionally, a comprehensive design document outlines a strategic refactoring of core Git domain objects, aiming to improve modularity and reduce the 'god object' anti-pattern in Carson::Runtime. The proposed architectural changes, including the introduction of Carson::Remote, Carson::PullRequest, and Carson::Branch objects, are well-articulated and follow sound design principles. However, there are a few minor issues identified in the linter configurations and the Ruby code snippets within the design document that should be addressed for correctness and clarity.
| - rubocop-performance | ||
|
|
||
| AllCops: | ||
| TargetRubyVersion: 4.0 # Parse and lint as Ruby 4.0. |
There was a problem hiding this comment.
| "javascript": { | ||
| "formatter": { | ||
| "indentStyle": "tab" | ||
| } |
| "css": { | ||
| "formatter": { | ||
| "indentStyle": "tab" | ||
| } |
|
|
||
| def force_push_with_lease!( branch: ) | ||
| # Returns self. Raises Remote::Error on failure. | ||
| end |
There was a problem hiding this comment.
The Ruby code block for Carson::Remote is missing an end keyword for the class definition. This would result in a syntax error if executed. Please ensure all code snippets are syntactically correct.
| end | |
| def force_push_with_lease!( branch: ) | |
| # Returns self. Raises Remote::Error on failure. | |
| end | |
| end |
|
|
||
| def review_decision | ||
| # Returns :approved, :changes_requested, :review_required, or :none. | ||
| end |
There was a problem hiding this comment.
The Ruby code block for Carson::PullRequest is missing an end keyword for the class definition. This would result in a syntax error if executed. Please ensure all code snippets are syntactically correct.
| end | |
| def review_decision | |
| # Returns :approved, :changes_requested, :review_required, or :none. | |
| end | |
| end |
|
|
||
| def self.absorbed( runtime: ) | ||
| # Returns array of Branch instances. Fully merged into main. | ||
| end |
There was a problem hiding this comment.
The Ruby code block for Carson::Branch is missing an end keyword for the class definition. This would result in a syntax error if executed. Please ensure all code snippets are syntactically correct.
| end | |
| def self.absorbed( runtime: ) | |
| # Returns array of Branch instances. Fully merged into main. | |
| end | |
| end |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace push_branch! and force_push_with_lease! private methods with calls to Carson::Remote#push! and Carson::Remote#force_push_with_lease!. Update the contested-push test to exercise Remote::Error directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all six call sites of repository_coordinates in review.rb, audit.rb, govern.rb, and prune.rb with Remote.new construction, then delete the method from data_access.rb. Update test stubs accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduces PullRequest domain object with find_open, for_branch, and open_for_branch? class methods, plus Error subclass with recovery contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds create! class method that raises PullRequest::Error with recovery hint on failure, and default_title helper for branch-name humanisation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds instance methods for merging a PR (with error contract), querying CI check status, and querying the review decision on a PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds merged_for_branch class method that queries the GitHub REST API for closed PRs matching an exact head SHA, enabling evidence-based force deletion in prune. Accepts owner, repo, and main_branch as parameters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all PR-related private methods in deliver.rb (find_or_create_pr!, find_existing_pr, create_pr!, default_pr_title, check_pr_ci, check_pr_review, merge_pr!) with calls to the PullRequest domain object. Update the test_default_pr_title_from_branch_name test to call PullRequest.default_title directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace current_pull_request_for_branch, branch_has_open_pr?, and merged_pr_for_branch with thin adapters that delegate to PullRequest domain object methods. Add title attr_reader to PullRequest so the review gate report can surface PR titles without raw gh calls. Update tests to reflect the adapter's delegation behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI ruby_indentation_guard caught space-based alignment on lines 219 and 230 where .map chained after .reject used spaces instead of tabs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
No description provided.