Add ref to list contents to list contents for branch, commit etc #44
Conversation
WalkthroughThe changes introduce an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant GitHub
Client->>Adapter: listRepositoryContents(owner, repo, path, ref)
Adapter->>GitHub: listRepositoryContents(owner, repo, path, ref)
GitHub->>GitHub: Construct API URL (add ?ref=ref if provided)
GitHub->>GitHub: Fetch repository contents from API
GitHub-->>Adapter: Return contents array
Adapter-->>Client: Return contents array
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/VCS/Adapter.php(1 hunks)src/VCS/Adapter/Git/GitHub.php(1 hunks)tests/VCS/Base.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/VCS/Adapter.php (1)
src/VCS/Adapter/Git/GitHub.php (1)
listRepositoryContents(208-243)
tests/VCS/Base.php (2)
src/VCS/Adapter.php (1)
listRepositoryContents(233-233)src/VCS/Adapter/Git/GitHub.php (1)
listRepositoryContents(208-243)
src/VCS/Adapter/Git/GitHub.php (1)
src/VCS/Adapter.php (1)
listRepositoryContents(233-233)
🔇 Additional comments (3)
src/VCS/Adapter.php (1)
230-233: LGTM! Clean interface update with proper documentation.The addition of the optional
$refparameter is well-implemented:
- Maintains backward compatibility with default empty string value
- PHPDoc clearly documents the parameter's purpose
- Consistent with GitHub API conventions for specifying git references
tests/VCS/Base.php (1)
93-97: Good test coverage for the new ref parameter.The test case appropriately covers the new functionality by:
- Testing with a common branch reference ('main')
- Verifying the method returns a non-empty array
- Following consistent assertion patterns with existing tests
src/VCS/Adapter/Git/GitHub.php (1)
205-208: Implementation correctly follows GitHub API conventions.The method signature and documentation updates align well with the abstract interface and GitHub's Contents API requirements.
| if (!empty($ref)) { | ||
| $url .= "?ref=$ref"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL encoding for the ref parameter to handle special characters.
The implementation correctly adds the ref query parameter, but the value should be URL encoded to handle branch names with special characters (e.g., "feature/new-feature") or tag names with special characters.
Apply this diff to fix the URL encoding:
- if (!empty($ref)) {
- $url .= "?ref=$ref";
- }
+ if (!empty($ref)) {
+ $url .= "?ref=" . urlencode($ref);
+ }🤖 Prompt for AI Agents
In src/VCS/Adapter/Git/GitHub.php around lines 214 to 216, the ref parameter is
appended to the URL without URL encoding, which can cause issues with special
characters in branch or tag names. Fix this by applying URL encoding to the $ref
value using a function like urlencode() before appending it to the URL query
string.
Summary by CodeRabbit