liam icon indicating copy to clipboard operation
liam copied to clipboard

chore: replace `pnpm run` with `concurrently` to maintain cross-platform tolerance

Open samuel871211 opened this issue 8 months ago • 5 comments

Issue

  • resolve: https://github.com/liam-hq/liam/issues/1212

Why is this change needed?

What would you like reviewers to focus on?

Testing Verification

What was done

🤖 Generated by PR Agent at d5696a05cdb7b6dc1ac038570259e5a2f2f04d9d

  • Replaced pnpm run with concurrently for cross-platform compatibility.
  • Updated all package.json scripts to use concurrently for pattern matching.
  • Added concurrently as a new dependency in the root package.json.
  • Updated pnpm-lock.yaml to include concurrently and its dependencies.

Detailed Changes

Relevant files
Enhancement
12 files
package.json
Updated scripts to use concurrently for cross-platform compatibility
+4/-4     
package.json
Replaced `pnpm run` with `concurrently` in scripts             
+2/-2     
package.json
Updated CLI scripts to use `concurrently`                               
+5/-5     
package.json
Modified scripts to adopt `concurrently` for pattern matching
+3/-3     
package.json
Replaced `pnpm run` with `concurrently` in database scripts
+3/-3     
package.json
Updated E2E scripts to use `concurrently`                               
+2/-2     
package.json
Adopted `concurrently` for script execution in ERD core   
+4/-4     
package.json
Updated scripts to use `concurrently` for Figma-to-CSS tasks
+2/-2     
package.json
Replaced `pnpm run` with `concurrently` in GitHub package scripts
+2/-2     
package.json
Updated job-related scripts to use `concurrently`               
+2/-2     
package.json
Modified scripts to use `concurrently` for prompt testing
+2/-2     
package.json
Updated UI package scripts to use `concurrently`                 
+4/-4     
Dependencies
2 files
package.json
Added `concurrently` dependency and updated scripts           
+3/-2     
pnpm-lock.yaml
Updated lockfile to include `concurrently` and its dependencies
+24/-0   

Additional Notes

I do modified pnpm-lock.yaml with only concurrently and it's dependency without modifying other package's version, I believe this will not change the functionality of other packages.


Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • samuel871211 avatar Apr 07 '25 13:04 samuel871211

    ⚠️ No Changeset found

    Latest commit: d2a2a09e798b5851fd4decd396f8920cea7376f9

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    changeset-bot[bot] avatar Apr 07 '25 13:04 changeset-bot[bot]

    @samuel871211 is attempting to deploy a commit to the ROUTE06 Core Team on Vercel.

    A member of the Team first needs to authorize it.

    vercel[bot] avatar Apr 07 '25 13:04 vercel[bot]

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    1212 - Fully compliant

    Compliant requirements:

    • Fix the issue where "pnpm run '/^fmt:.*/'" doesn't work on Windows • Implement a cross-platform compatible solution • Maintain the same functionality where scripts with matching patterns can be run together

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix script pattern matching
    Suggestion Impact:The commit implemented exactly what was suggested - changing the pattern from 'pnpm:supabase:gen*' to 'pnpm:supabase:gen:*' to correctly match script names with the proper colon

    code diff:

    -    "supabase:gen": "concurrently \"pnpm:supabase:gen*\"",
    +    "supabase:gen": "concurrently \"pnpm:supabase:gen:*\"",
    

    The pattern for concurrently should include a colon after "gen" to match the
    script naming pattern. Currently it would run all scripts starting with
    "supabase:gen" (missing the colon), which might not match your intended scripts.

    frontend/packages/db/package.json [26]

    -"supabase:gen": "concurrently \"pnpm:supabase:gen*\"",
    +"supabase:gen": "concurrently \"pnpm:supabase:gen:*\"",
    

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    __

    Why: The pattern "pnpm:supabase:gen*" is missing a colon after "gen", which would incorrectly match script names. The correct pattern should be "pnpm:supabase:gen:*" to properly match scripts like "supabase:gen:schema_sql" and "supabase:gen:types".

    Medium
    • [ ] Update

    https://github.com/liam-hq/liam/pull/1216#issuecomment-2783330707

    I'm not sure whether to run the changeset or not, if it;s required, please run the changeset for me, thank you.

    samuel871211 avatar Apr 07 '25 13:04 samuel871211

    Hi @hoshinotsuyoshi I am not sure if this error is related to this PR Could you take a look at this error? I think it's related to this commit

    samuel871211 avatar Apr 07 '25 14:04 samuel871211

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    liam-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 0:25am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview Apr 8, 2025 0:25am
    1 Skipped Deployment
    Name Status Preview Comments Updated (UTC)
    liam-docs ⬜️ Ignored (Inspect) Visit Preview Apr 8, 2025 0:25am

    vercel[bot] avatar Apr 08 '25 00:04 vercel[bot]

    @samuel871211 That's the problem that happens when it's a PR from a fork. Thanks, you can ignore it for now.

    MH4GF avatar Apr 08 '25 00:04 MH4GF