-
Notifications
You must be signed in to change notification settings - Fork 0
ci: simplify #31
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
ci: simplify #31
Conversation
|
Warning Rate limit exceeded@loks0n has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
WalkthroughThis pull request removes three existing GitHub Actions workflows (codeql-analysis.yml, linter.yml, tests.yml) and adds a new consolidated CI workflow at .github/workflows/ci.yml with four jobs: format, analyze, unit-tests, and e2e-tests. Relevant jobs check out the repository (some with recursive submodules), run Composer/PHP tools inside Docker, and the e2e job builds a Docker image with Buildx and runs docker-compose-based end-to-end tests. composer.json is updated to replace the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
3-3: Consider addingpushtrigger for the main branch.The workflow only triggers on
pull_request, meaning CI won't run when PRs are merged tomain. This could allow broken code to entermainif a PR becomes stale or if merge conflicts introduce issues.🔎 Proposed fix
-on: [pull_request] +on: + pull_request: + push: + branches: [main]
66-69: Consider replacing fixed sleep with a health check.A fixed
sleep 10is fragile—the DNS server might take longer to start in some cases, or the wait could be unnecessarily long. A retry loop with a health check would be more robust.🔎 Proposed improvement
- name: Start DNS Server run: | docker compose up -d - sleep 10 + timeout 60 sh -c 'until docker compose exec -T dns-server php -r "echo \"ready\";" 2>/dev/null; do sleep 2; done'Alternatively, if the DNS server exposes a health endpoint or you can use
digto query it, that would be even more reliable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/codeql-analysis.yml.github/workflows/linter.yml.github/workflows/tests.ymlcomposer.json
💤 Files with no reviewable changes (3)
- .github/workflows/linter.yml
- .github/workflows/tests.yml
- .github/workflows/codeql-analysis.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (5)
composer.json (1)
10-11: LGTM!The new scripts are well-defined and correctly reference tools already present in
require-dev. The--testflag for Pint ensures format checking without modifying files, and PHPStan at level 8 provides strict static analysis..github/workflows/ci.yml (4)
6-16: LGTM!The format job is well-structured. Using the official
composer:2.7image ensures a consistent environment.
30-42: LGTM!The unit-tests job correctly checks out submodules and runs only the unit test suite.
53-64: LGTM!Good use of Docker Buildx with GHA caching (
cache-fromandcache-to). This will significantly speed up subsequent builds.
18-28: The repository has no submodules configured. The analyze job does not need to checkout submodules because none exist in the project. Thesubmodules: recursiveflags in other jobs are benign and do not affect the analyze job's ability to run PHPStan.Likely an incorrect or invalid review comment.
e5b60c1 to
1f853e0
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.