Skip to content

fix: execute inside of matrix package not root #180

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.

Sign up for GitHub

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

Merged
aimensahnoun merged 1 commit into from
Nov 4, 2024
Merged

fix: execute inside of matrix package not root #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/npm-publish.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
- name: Check release type 🕵️‍♀️
id: check-release
run: |
RELEASE_TYPE=$(./scripts/check-release-type.sh)
RELEASE_TYPE=$(npm run check-release-type --workspace=${{ matrix.package }} --silent)
echo "release-type=$RELEASE_TYPE" >> $GITHUB_OUTPUT

- name: Publish stable version 📦
Expand All @@ -53,7 +53,7 @@ jobs:

- name: Publish next version 📦
if: steps.check-release.outputs.release-type == 'next'
run: ./scripts/publish-next-release.sh
run: npm run publish-next-release --workspace=${{ matrix.package }}
env:
NODE_AUTH_TOKEN: ${{ secrets.REQUEST_BOT_NPM_TOKEN }}
VITE_WEB3MODAL_PROJECT_ID: ${{ secrets.VITE_WEB3MODAL_PROJECT_ID }}
3 changes: 2 additions & 1 deletion packages/add-stakeholder/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"clean": "rm -rf dist && rm -rf .svelte-kit",
"is-release-needed": "bash ../../scripts/is-release-needed.sh"
"check-release-type": "bash ../../scripts/check-release-type.sh",
"publish-next-release": "bash ../../scripts/publish-next-release.sh"
},
"exports": {
".": {
Expand Down
3 changes: 2 additions & 1 deletion packages/create-invoice-form/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"clean": "rm -rf dist && rm -rf .svelte-kit",
"is-release-needed": "bash ../../scripts/is-release-needed.sh"
"check-release-type": "bash ../../scripts/check-release-type.sh",
"publish-next-release": "bash ../../scripts/publish-next-release.sh"
Comment on lines +15 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Document release process changes

Consider adding comments in the package.json or updating the repository's documentation to explain:

  • The purpose of check-release-type and publish-next-release
  • When and how these scripts should be used
  • The relationship between these scripts and the CI/CD pipeline

},
"exports": {
".": {
Expand Down
3 changes: 2 additions & 1 deletion packages/invoice-dashboard/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"clean": "rm -rf dist && rm -rf .svelte-kit",
"is-release-needed": "bash ../../scripts/is-release-needed.sh"
"check-release-type": "bash ../../scripts/check-release-type.sh",
"publish-next-release": "bash ../../scripts/publish-next-release.sh"
Comment on lines +15 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider improving script robustness and cross-platform compatibility.

While the current implementation works, consider these improvements:

  1. Use cross-platform npm scripts instead of direct bash calls
  2. Add script existence validation
  3. Implement proper error handling

Example improvement:

-    "check-release-type": "bash ../../scripts/check-release-type.sh",
-    "publish-next-release": "bash ../../scripts/publish-next-release.sh"
+    "check-release-type": "node ../../scripts/check-release-type.js",
+    "publish-next-release": "node ../../scripts/publish-next-release.js"

This would:

  • Improve Windows compatibility
  • Allow better error handling through Node.js
  • Enable TypeScript/JavaScript usage for better maintainability

Committable suggestion skipped: line range outside the PR's diff.

},
"exports": {
".": {
Expand Down
3 changes: 2 additions & 1 deletion packages/payment-widget/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"clean": "rm -rf dist && rm -rf .svelte-kit",
"is-release-needed": "bash ../../scripts/is-release-needed.sh"
"check-release-type": "bash ../../scripts/check-release-type.sh",
"publish-next-release": "bash ../../scripts/publish-next-release.sh"
Comment on lines +15 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Scripts need executable permissions

The referenced shell scripts exist and are consistently used across all packages, but they lack executable permissions:

  • scripts/check-release-type.sh (current: rw-r--r--)
  • scripts/publish-next-release.sh (current: rw-r--r--)

Please add executable permissions to these scripts:

chmod +x scripts/check-release-type.sh scripts/publish-next-release.sh
🔗 Analysis chain

LGTM! Verify referenced shell scripts exist.

The script changes correctly use relative paths to ensure execution from the package context, which aligns with the PR objective.

Let's verify the referenced shell scripts exist and have proper permissions:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and permissions of the referenced shell scripts

# Test: Check if the scripts exist and are executable
ls -l scripts/check-release-type.sh scripts/publish-next-release.sh

# Test: Verify these scripts are consistently used across all packages
fd -t f "package.json" packages/ -x grep -l "check-release-type\|publish-next-release"

Length of output: 471

Script:

#!/bin/bash
# Let's verify if the scripts have consistent usage across all packages by checking their content
for pkg in packages/*/package.json; do
  echo "=== $pkg ==="
  grep -A 2 "check-release-type\|publish-next-release" "$pkg"
done

Length of output: 1584

},
"exports": {
".": {
Expand Down