-
Notifications
You must be signed in to change notification settings - Fork 189
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: add jobs requirements to avoid consuming time with invalid linters #2864
Conversation
WalkthroughOhayo, sensei! The CI workflow configuration has been updated to introduce job dependencies. The Changes
Sequence DiagramsequenceDiagram
participant fmt
participant cairofmt
participant build
participant ensure-wasm
participant clippy
participant docs
fmt ->> fmt: Run formatting checks
cairofmt ->> cairofmt: Run Cairo formatting checks
fmt -->> build: Dependency
cairofmt -->> build: Dependency
fmt -->> ensure-wasm: Dependency
cairofmt -->> ensure-wasm: Dependency
fmt -->> clippy: Dependency
cairofmt -->> clippy: Dependency
fmt -->> docs: Dependency
cairofmt -->> docs: Dependency
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
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/ci.yml (1)
16-16
: Consider optimizing the job dependency graph furtherWhile the current changes are good, we could potentially optimize the pipeline further:
The
clippy
job could run in parallel withbuild
since they don't depend on each other's outputs. This would reduce the total pipeline execution time.The
docs
job could potentially run in parallel with other jobs since it's independent of the build artifacts.Here's a visualization of the suggested dependency graph:
fmt & cairofmt │ ├─────┬─────┬─────┐ │ │ │ │ build docs clippy ensure-wasm │ └─> ensure-docker │ └─> test
Also applies to: 59-59, 158-158, 177-177
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml
(4 hunks)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
16-16
: Ohayo sensei! The job dependency changes look good! 🎋Adding
fmt
andcairofmt
as prerequisites for the build, ensure-wasm, clippy, and docs jobs is a smart optimization. This ensures we fail fast on formatting issues before consuming compute resources on subsequent tasks.Also applies to: 59-59, 158-158, 177-177
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2864 +/- ##
=======================================
Coverage 55.82% 55.82%
=======================================
Files 446 446
Lines 57730 57730
=======================================
Hits 32228 32228
Misses 25502 25502 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit