-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
fix: Reverted changes for skip client cache #37782
Conversation
WalkthroughThe pull request includes significant updates to the client build workflow configuration in GitHub Actions. Key changes involve the adoption of the 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
|
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.
git lfs install | ||
git lfs migrate import --everything --yes |
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.
🛠️ Refactor suggestion
Remove redundant Git LFS installation
The git lfs install
command at line 215 is redundant as it's already executed earlier in the step. This duplication may cause unnecessary processing.
Remove the redundant command:
- git lfs install
git lfs migrate import --everything --yes
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
git lfs install | |
git lfs migrate import --everything --yes | |
git lfs migrate import --everything --yes |
- name: Get changed files in the client folder | ||
id: changed-files-specific | ||
run: echo "any_changed=true" >> "$GITHUB_OUTPUT" | ||
uses: tj-actions/changed-files@v41 | ||
with: | ||
files: "app/client/**" | ||
|
||
#- name: Run step if any file(s) in the client folder change | ||
# if: steps.changed-files-specific.outputs.any_changed == 'true' | ||
# env: | ||
# ALL_CHANGED_FILES: ${{ steps.changed-files-specific.outputs.all_changed_files }} | ||
# run: | | ||
# echo "One or more files in the server folder has changed." | ||
# echo "List all the files that have changed: $ALL_CHANGED_FILES" | ||
# - name: Updating the client changed file variable | ||
# id: changed-files-specific | ||
# run: echo "any_changed=true" >> "$GITHUB_OUTPUT" | ||
|
||
- name: Run step if any file(s) in the client folder change | ||
if: steps.changed-files-specific.outputs.any_changed == 'true' | ||
env: | ||
ALL_CHANGED_FILES: ${{ steps.changed-files-specific.outputs.all_changed_files }} | ||
run: | | ||
echo "One or more files in the server folder has changed." | ||
echo "List all the files that have changed: $ALL_CHANGED_FILES" |
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.
💡 Codebase verification
Version inconsistency detected in tj-actions/changed-files usage
The action is used with different versions across workflows:
- client-build.yml: v41
- server-build.yml: v44
- ci-client-cyclic-deps-check.yml: v41
Please update all workflows to use the latest version (v44) for consistency and to benefit from the latest features and bug fixes.
Locations that need updates:
.github/workflows/client-build.yml:71
.github/workflows/ci-client-cyclic-deps-check.yml:32
🔗 Analysis chain
LGTM: Improved file change detection implementation
The switch to tj-actions/changed-files@v41
is a good improvement over the manual echo approach. The implementation correctly uses the action's outputs for conditional execution.
Let's verify the action's configuration:
Let me analyze the configuration details of the changed-files action across different workflows to ensure consistency and identify any potential version mismatches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify tj-actions/changed-files configuration
# Check if the action is properly configured in other workflows
rg -p "tj-actions/changed-files" .github/workflows/
Length of output: 312
Script:
#!/bin/bash
# Check the configuration details of changed-files action across workflows
cat .github/workflows/client-build.yml .github/workflows/server-build.yml .github/workflows/ci-client-cyclic-deps-check.yml | awk '/tj-actions\/changed-files/,/with:/ { p=1 } p; /^[^ ]/ { p=0 }'
Length of output: 26401
Description
Added command for migration
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