Updated deps and fixed coding standards.#269
Conversation
📝 WalkthroughWalkthroughCI workflow enhanced with composer dependency security audit. composer.json updated with newer dependency versions (Symfony, phpstan, phpunit, rector) and new audit/plugin configuration entries. Exception handling improved in repository and test classes to propagate previous exception context for better error diagnostics. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #269 +/- ##
==========================================
- Coverage 96.93% 96.91% -0.03%
==========================================
Files 6 6
Lines 424 421 -3
==========================================
- Hits 411 408 -3
Misses 13 13 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Git/ArtifactGitRepository.php (1)
284-294:⚠️ Potential issue | 🟡 MinorConsider passing
$e2instead of$e1for better error context.The code catches
$e1from the branch verification (line 284) and$e2from the tag verification (line 288). When throwingBranchNotFoundExceptionat line 293, you pass$e1, but$e2is the most recent exception that definitively confirms the value is neither a branch nor a tag.Passing
$e2would provide more relevant context about why the detachment source couldn't be resolved.Proposed fix
- throw new BranchNotFoundException('Unable to determine a detachment source', $commit_hash, $e1); + throw new BranchNotFoundException('Unable to determine a detachment source', $commit_hash, $e2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Git/ArtifactGitRepository.php` around lines 284 - 294, The catch block currently throws BranchNotFoundException using the outer exception variable $e1, but the inner GitException $e2 is the one that proves the ref is neither a tag nor a branch; update the throw to pass $e2 instead of $e1 so the exception carries the most relevant error context. Locate the try/catch around execute(['show-ref', '--verify', 'refs/tags/' . $branch]) and the inner catch (GitException $e2), and change the thrown BranchNotFoundException(...) to use $e2 while keeping the same message and $commit_hash retrieval via execute(['rev-parse', 'HEAD'])[0] ?? ''.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Git/ArtifactGitRepository.php`:
- Around line 284-294: The catch block currently throws BranchNotFoundException
using the outer exception variable $e1, but the inner GitException $e2 is the
one that proves the ref is neither a tag nor a branch; update the throw to pass
$e2 instead of $e1 so the exception carries the most relevant error context.
Locate the try/catch around execute(['show-ref', '--verify', 'refs/tags/' .
$branch]) and the inner catch (GitException $e2), and change the thrown
BranchNotFoundException(...) to use $e2 while keeping the same message and
$commit_hash retrieval via execute(['rev-parse', 'HEAD'])[0] ?? ''.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4d85c0f-1ddf-4311-85b8-a673b1ca7926
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/test-php.ymlcomposer.jsonsrc/Git/ArtifactGitRepository.phptests/Traits/ConsoleTrait.php
Summary by CodeRabbit
New Features
Chores