(fix) GitHub Actions fixes from repo refactor #14723
Conversation
Revving Node version
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe pull request includes updates to several GitHub Actions workflows and documentation for the Changes
Possibly related PRs
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (8)
.github/workflows/publish-platform-package.yaml (1)
Line range hint
1-40: Consider enhancing build reliability and safetyWhile reviewing the workflow context, I noticed two potential improvements:
- Consider using
--frozen-lockfilewith pnpm install to ensure consistent builds:- run: pnpm install -r --no-frozen-lockfile + run: pnpm install -r --frozen-lockfile
- The
--no-git-checksflag in the publish command bypasses important safety checks. Consider removing it unless absolutely necessary:- run: pnpm publish platform --no-git-checks + run: pnpm publish platform.github/actions/git-diff-on-components/README.md (2)
18-24: Add language specifier to the code blockThe code block should specify
yamlas the language for proper syntax highlighting.-``` +```yaml ... with: format: json<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 19-19: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `42-45`: **Improve command block formatting** The command block should follow markdown best practices: 1. Specify the language as `bash` or `shell` 2. Remove `$` signs from commands as per markdown guidelines ```diff -``` -$ cd .github/actions/git-diff-on-components/ -$ pnpm i && pnpm run build -``` +```bash +cd .github/actions/git-diff-on-components/ +pnpm i && pnpm run build +```🧰 Tools
🪛 Markdownlint (0.35.0)
43-43: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
44-44: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
42-42: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
.github/workflows/publish-packages.yaml (1)
Line range hint
29-31: Update deprecatedset-outputcommandThe workflow uses the deprecated
set-outputcommand which will be removed in a future version of GitHub Actions.Update the command to use the new syntax:
- echo "::set-output name=pnpm_cache_dir::$(pnpm store path)" + echo "pnpm_cache_dir=$(pnpm store path)" >> $GITHUB_OUTPUTReference: GitHub Actions: Deprecating save-state and set-output commands
.github/workflows/pull-request-checks.yaml (2)
81-81: Consider pinning to a specific Node.js versionUsing only the major version (
18) could lead to unexpected behavior if minor versions introduce breaking changes. Consider pinning to a specific version (e.g.,18.18.0) for better reproducibility.- node-version: 18 + node-version: 18.18.0
Line range hint
84-94: Optimize changed files retrievalThe workflow currently calls
get-changed-filesaction twice with different formats. Consider consolidating into a single step and transforming the format as needed in the subsequent steps.Example optimization:
- - name: Get Changed Files (space-separated) - id: changed_files_space - uses: Ana06/[email protected] - with: - format: 'space-delimited' - - name: Lint changed files - run: pnpm exec eslint ${{ steps.changed_files_space.outputs.added_modified }} ${{ steps.changed_files_space.outputs.renamed }} - - name: Get Changed Files (comma-separated) - id: changed_files + - name: Get Changed Files + id: changed_files uses: Ana06/[email protected] with: format: 'csv' + - name: Lint changed files + run: pnpm exec eslint $(echo "${{ steps.changed_files.outputs.added_modified }},${{ steps.changed_files.outputs.renamed }}" | tr ',' ' ').github/workflows/components-pr.yaml (1)
Line range hint
1-1: Consider consolidating duplicate workflows.The TODO comment at the top of the file suggests combining this with
publish-components.yml. This would be a good follow-up task to reduce duplication and maintenance overhead.Would you like me to help create a GitHub issue to track the workflow consolidation task?
.github/workflows/publish-components.yaml (1)
Line range hint
11-11: Consider pinning GitHub Actions to exact commit hashes.For enhanced security, consider pinning the GitHub Actions to exact commit hashes instead of version tags:
actions/[email protected]Ana06/[email protected]- Other actions in the workflow
This prevents potential supply chain attacks through compromised action versions.
Also applies to: 72-72, 139-139
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.github/actions/git-diff-on-components/README.md(2 hunks).github/actions/git-diff-on-components/package.json(1 hunks).github/workflows/components-pr.yaml(2 hunks).github/workflows/publish-components.yaml(3 hunks).github/workflows/publish-packages.yaml(1 hunks).github/workflows/publish-platform-package.yaml(1 hunks).github/workflows/pull-request-checks.yaml(2 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
.github/actions/git-diff-on-components/README.md
19-19: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
43-43: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
44-44: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
42-42: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (12)
.github/actions/git-diff-on-components/package.json (2)
8-8: LGTM! Verify pnpm availability in workflows
The change from npm to pnpm is consistent with the PR objectives. However, let's verify the setup in the workflows that use this action.
✅ Verification successful
LGTM! The pnpm setup is properly configured in workflows
The change from npm to pnpm in the git-diff-on-components action's build script is safe. The workflows that use this action have proper pnpm setup:
- All relevant workflows include
pnpm/[email protected]with version 9.14.2 - They properly configure pnpm caching
- Node.js setup is configured with pnpm cache
- The workflows install dependencies using pnpm commands
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if workflows using this action have pnpm setup steps
rg -l "uses: .*/git-diff-on-components@" | xargs rg "pnpm|actions/setup-node"
Length of output: 3255
Script:
#!/bin/bash
# Let's check the workflow files specifically
fd "workflow" .github/workflows/ | xargs rg "pnpm|actions/setup-node|git-diff-on-components"
Length of output: 3268
Script:
#!/bin/bash
# Let's try a more specific search in the workflows directory
find .github/workflows -type f -exec cat {} \;
Length of output: 30113
8-8: Verify documentation updates for pnpm
Since we're changing the build system to pnpm, ensure that the action's documentation (README.md, contribution guidelines) reflects this change.
✅ Verification successful
Let me verify if there are any other documentation files or contribution guidelines that might need updating.
Let me correct the command to properly search for documentation files.
Let me check the content of the README.md file to verify if it needs updating regarding the build system.
Documentation is already up to date with pnpm
The README.md already includes the correct pnpm build instructions in the "Build" section:
$ cd .github/actions/git-diff-on-components/
$ pnpm i && pnpm run build
No updates are needed as the documentation accurately reflects the current build system using pnpm.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if documentation mentions npm vs pnpm
fd README.md .github/actions/git-diff-on-components | xargs rg -i "npm|pnpm"
Length of output: 105
Script:
#!/bin/bash
# Check for any documentation files and contribution guidelines
fd -e md .github/actions/git-diff-on-components
# Also check if there are any other files that might contain documentation
fd CONTRIBUTING DEVELOP DEVELOPMENT SETUP .github/actions/git-diff-on-components
Length of output: 754
Script:
#!/bin/bash
# Search for markdown files in the component directory
fd -e md . .github/actions/git-diff-on-components
# Also check for any setup or contribution related files
fd -i "contributing|develop|setup|readme" . .github/actions/git-diff-on-components
Length of output: 75890
Script:
#!/bin/bash
# Check the content of the README.md file
cat .github/actions/git-diff-on-components/README.md
Length of output: 1227
.github/workflows/publish-platform-package.yaml (1)
27-27: LGTM! Build command correctly updated to use pnpm
The change from npm to pnpm aligns with the workflow's setup and the broader repository refactor.
.github/actions/git-diff-on-components/README.md (1)
39-41: LGTM! Important note about dist files
Good addition of the note about pushing dist files. This is crucial information for action development and testing.
.github/workflows/publish-packages.yaml (1)
48-48: LGTM! Build command updated to use pnpm
The change from npm run build to pnpm run build is correct and consistent with the workflow's use of pnpm as the package manager.
.github/workflows/pull-request-checks.yaml (2)
53-60: Well-documented configuration for git history
The comments clearly explain why full history is needed for proper git diff functionality. This is the correct approach for accurately determining changed files.
62-63: Verify pnpm version
Let's verify if 9.14.2 is the latest stable version of pnpm.
✅ Verification successful
pnpm version is up to date
The installed pnpm version 9.14.2 matches the latest stable version from the npm registry.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest stable version of pnpm
curl -s https://registry.npmjs.org/pnpm/latest | jq '.version'
Length of output: 71
.github/workflows/components-pr.yaml (2)
76-76: LGTM! Build command updated for pnpm.
The change from npm to pnpm is consistent with the PR objectives to standardize package management.
165-165: LGTM! Build command updated for pnpm.
The change from npm to pnpm is consistent with the PR objectives to standardize package management.
.github/workflows/publish-components.yaml (3)
38-38: LGTM: Build command updated consistently with pnpm transition.
The change to pnpm run build aligns with the workflow's pnpm setup and caching configuration.
140-142: LGTM: Node.js version upgrade is consistent across jobs.
The upgrade to Node.js 18 maintains consistency with the publish-components job.
157-157: LGTM: Build command updated with proper output handling.
The change to pnpm run build with output redirection to files.txt maintains the workflow's TypeScript component processing functionality.
Let's verify the build script configuration:
✅ Verification successful
LGTM: Build output redirection is correctly used for TypeScript component publishing
The output redirection to files.txt is essential for the TypeScript component publishing workflow as it captures the compiled file paths which are later used to match and publish the corresponding TypeScript components. The workflow reads this file to identify which compiled .mjs files correspond to the changed .ts files, making the redirection a critical part of the publishing process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the build script is properly configured in package.json
fd package.json --exec jq -r 'select(.scripts != null) | select(.scripts.build != null) | .scripts.build'
Length of output: 242
Script:
#!/bin/bash
# Let's check the package.json files to understand which one corresponds to the TypeScript components
fd package.json --exec sh -c 'echo "=== $1 ==="; cat "$1" | jq -r "select(.name != null) | {name, scripts}"' sh {}
Length of output: 100123
Script:
#!/bin/bash
# Let's check the workflow file to understand the context of the build command
cat .github/workflows/publish-components.yaml
Length of output: 12439
| $ pnpm i && pnpm run build |
There was a problem hiding this comment.
💡 Codebase verification
Package manager inconsistency detected
The repository currently has mixed package manager configuration:
- Both
package-lock.jsonandpnpm-lock.yamlfiles are present - Multiple workflow files still use npm commands:
.github/workflows/publish-packages.yaml.github/workflows/publish-platform-package.yaml.github/workflows/publish-components.yaml.github/workflows/components-pr.yaml.github/workflows/pipedream-sdk-test.yaml.github/workflows/publish-marketplace-content.yaml.github/workflows/pull-request-checks.yaml
🔗 Analysis chain
Verify package manager transition
The change from npm to pnpm should be consistent across the repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package manager configuration and lock files
# Check for remaining npm references in workflow files
echo "Checking for npm references in workflows:"
rg -l 'npm (install|ci|run|build)' .github/workflows/
# Check for package manager lock files
echo -e "\nChecking lock files:"
ls -la package-lock.json pnpm-lock.yaml 2>/dev/null
Length of output: 687
🧰 Tools
🪛 Markdownlint (0.35.0)
44-44: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
| node-version: 18 | ||
| registry-url: https://registry.npmjs.org/ | ||
| cache: 'pnpm' |
There was a problem hiding this comment.
💡 Codebase verification
The Node.js 18 upgrade is appropriate, but there's an inconsistency in the workflow files:
- All workflows are using Node.js 18 except
publish-marketplace-content.yamlwhich is still on Node.js 14 - Package.json indicates
>=18.0.0as the Node.js requirement
🔗 Analysis chain
LGTM: Node.js version upgrade is necessary and well-timed.
The upgrade from Node.js 14 to 18 is essential as v14 has reached end-of-life. Node.js 18 is currently in Active LTS status, making it a stable choice.
Let's verify Node.js version compatibility across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any Node.js version constraints in package.json files
fd package.json --exec jq -r 'select(.engines != null) | .engines.node'
Length of output: 87
Script:
#!/bin/bash
# Let's check for any specific Node.js version references in the workflow files
rg "node-version:" .github/workflows/ -A 2
Length of output: 2101
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Chores