-
Notifications
You must be signed in to change notification settings - Fork 33
feat: use composer's --no-security-blocking flag
#409
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
Conversation
WalkthroughAppends a standalone "check ci" line to README.md and adds the Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Composer as ramsey/composer-install@v3
note right of GH#ffefc2: integration-tests job
rect rgba(214,245,214,0.3)
GH->>Composer: run composer install\n(old: without --no-security-blocking)
end
rect rgba(255,239,194,0.3)
GH->>Composer: run composer install\n(new: with --no-security-blocking)
end
note right of Composer#d6f5d6: installer skips security-blocking checks when flag present
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
sounds best to me |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
95-101: Consider conditional application of the security flag for targeted testing.If the primary goal is to ensure the "lowest" supported versions work (as stated in the PR objective), the
--no-security-blockingflag could be applied conditionally only to those specific runs rather than globally to all dependency scenarios.This would reduce security exposure during testing while still achieving the PR's goal of validating the lowest-supported environment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/tests.yml (1)
100-100: Approved:--no-security-blockingis correctly configured.The
--no-security-blockingflag is a documented Composer option that allows installing packages with security advisories or that are abandoned. The ramsey/composer-install action accepts this flag via thecomposer-optionsinput parameter, and your usage is correct.The flag is applied globally to all matrix runs (default, lowest, highest dependencies). While this is broader than strictly necessary, it's a valid design choice for simplicity in CI testing scenarios. Note that COMPOSER_NO_SECURITY_BLOCKING takes precedence and forces all security blocking to be disabled, including abandoned package blocking.
For testing purposes, this approach is acceptable—security checks in CI are less critical than in production deployments.
|
i will wait for composer/composer#12612 |
|
@Chris53897 composer already released new version |
Removed the 'check ci' line from the README.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
=======================================
Coverage 88.38% 88.38%
=======================================
Files 20 20
Lines 878 878
=======================================
Hits 776 776
Misses 102 102 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
--no-security-blocking flag
|
Thank you @Chris53897 |
check ci for lowest run
With the new composer version the runs for older minor symfony version like 7.1 fail
Solutions
Summary by CodeRabbit
Documentation
Chores