chore: skipping file change #37774
chore: skipping file change #37774
Conversation
WalkthroughThe changes in the pull request involve modifications to the workflow configuration in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/client-build.yml (1)
Line range hint
92-93: Simplify conditional checksSince
steps.changed-files-specific.outputs.any_changedis now alwaystrue, the conditional checks throughout the workflow can be simplified. This would improve readability and maintenance.Example simplification for the conditions:
-if: steps.run_result.outputs.run_result != 'success' && (steps.changed-files-specific.outputs.any_changed == 'true' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') +if: steps.run_result.outputs.run_result != 'success' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || true)Or even better:
-if: steps.run_result.outputs.run_result != 'success' && (steps.changed-files-specific.outputs.any_changed == 'true' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') +if: steps.run_result.outputs.run_result != 'success'Also applies to: 106-107, 121-122, 134-135, 147-148, 159-160, 171-172, 186-187, 199-200
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/client-build.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/client-build.yml (1)
69-77: Verify the impact of removing change detection
The change to bypass file detection and always run the client build could impact CI performance and resource usage. While this might be intentional for stability (as mentioned in PR #37768), we should verify:
- The performance impact on the CI pipeline
- Whether this is a temporary fix or permanent solution
Let's analyze the impact:
Consider alternatives if this is a permanent change:
- Implement more granular change detection
- Add a manual trigger to force builds when needed
- Use build caching strategies to minimize the impact
✅ Verification successful
Bypassing change detection is acceptable in this context
The modification to always set any_changed=true is actually well-aligned with the workflow's logic:
- The workflow already has explicit conditions for different events (
push,workflow_dispatch,schedule) - The cache restoration step for previous builds is properly handled
- The change detection bypass is consistently implemented with proper fallback conditions
The performance impact is mitigated by:
- Caching mechanisms for dependencies and builds
- Conditional steps that still respect event types
- Proper artifact handling for build outputs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check recent workflow runs to compare build times before and after this change
gh run list --workflow=client-build.yml --limit 10 --json durationInMinutes,status,conclusion \
| jq -r '.[] | "Duration: \(.durationInMinutes)min, Status: \(.status), Conclusion: \(.conclusion)"'
Length of output: 439
Script:
#!/bin/bash
# Let's check the workflow file to understand the dependencies and usage of the changed_files variable
cat .github/workflows/client-build.yml
Length of output: 12261
Script:
#!/bin/bash
# Let's also check for any related workflow files that might be affected
fd ".*\.ya?ml" .github/workflows/ --exec grep -l "changed-files"
Length of output: 184
Script:
#!/bin/bash
# Check if there are any conditional steps that depend on the changed_files output
rg "if: steps.changed-files" .github/workflows/
Length of output: 2443
Description
Skipping file change check for client build.
Fixes https://app.zenhub.com/workspaces/stability-pod-6690c4814e31602e25cab7fd/issues/gh/appsmithorg/appsmith/37768
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit