fix(ci): adjust sim conditional logic #3395
Conversation
📝 WalkthroughWalkthroughA new reusable GitHub Actions workflow Changes
Sequence DiagramsequenceDiagram
participant Caller as Workflow Caller
participant Reusable as Reusable Sim Workflow
participant Makefile as Makefile Target
Caller->>Reusable: Trigger with inputs
Reusable->>Reusable: Validate inputs
Reusable->>Reusable: Checkout code
Reusable->>Reusable: Setup Go
Reusable->>Reusable: Install dependencies
Reusable->>Makefile: Execute specified target
Possibly related PRs
Suggested Labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
gartnera
force-pushed
the
sim-tests-logic
branch
5 times, most recently
from
8b65aa9 to
e46031c
Compare
gartnera
force-pushed
the
sim-tests-logic
branch
5 times, most recently
from
ca7877b to
ea8e9f2
Compare
gartnera
force-pushed
the
sim-tests-logic
branch
from
b62e5e7 to
b4f4b97
Compare
This reverts commit b4f4b97.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/reusable-sim.yml (2)
21-22: Consider adding matrix configuration options.The strategy block is defined but only contains
fail-fast: false. Consider adding the ability to pass matrix configuration through inputs for more flexibility.strategy: fail-fast: false + matrix: ${{ fromJSON(inputs.matrix || '{}') }}This would require adding a new input:
matrix: description: 'Optional matrix configuration in JSON format' required: false type: string
35-37: Consider adding error handling for make command.The make command execution could benefit from additional error handling and output capture.
- name: Run ${{ inputs.make-target }} run: | + set -eo pipefail make ${{ inputs.make-target }} + if [ $? -ne 0 ]; then + echo "::error::Failed to execute make target: ${{ inputs.make-target }}" + exit 1 + fi.github/workflows/sim.yaml (2)
33-36: Consider caching changed files output.The
tj-actions/changed-filesaction could benefit from caching to improve workflow execution time, especially for large repositories.- name: Get changed files in x directory id: x-changes uses: tj-actions/changed-files@v45 with: files: x/** + cache: true
103-103: Consider adding timeout for sim-ok job.The sim-ok job should have a timeout to prevent hanging in case of issues.
if: ${{ !cancelled() }} + timeout-minutes: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/reusable-sim.yml(1 hunks).github/workflows/sim.yaml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: start-e2e-test / e2e
- GitHub Check: test-sim-nondeterminism / sim
- GitHub Check: gosec
- GitHub Check: build-and-test
- GitHub Check: build
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
.github/workflows/sim.yaml (1)
95-98: LGTM! Excellent use of reusable workflow.The implementation of the reusable workflow pattern is clean and well-structured. This will improve maintainability and reduce duplication.
Sim tests weren't automatically running on #3357 for changed files in
x/.Updates:
tj-actions/changed-fileswhich should better handle complicated branch history vsgit diffrelease/*branchRun with no change and no label: https://github.com/zeta-chain/node/actions/runs/12914920558
Run with change in
x/: https://github.com/zeta-chain/node/actions/runs/12914963998Summary by CodeRabbit
New Features
Workflow Updates
release/*branch pattern