Enable use as a Git dependency #1664
Conversation
WalkthroughThe pull request introduces significant enhancements to 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 (
|
alexeyr-ci
force-pushed
the
alexeyr/prepack-script
branch
from
58df136 to
722b351
Compare
alexeyr-ci
changed the title
alexeyr-ci
force-pushed
the
alexeyr/prepack-script
branch
2 times, most recently
from
ef8be23 to
5b811d7
Compare
![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
🧹 Nitpick comments (1)
package-scripts.yml (1)
17-29: Documentation needed for Git dependency usage.Since this change enables using the package as a Git dependency, please update the documentation to include:
- Instructions for installing the package as a Git dependency
- Any prerequisites or considerations when using Git dependencies
- Troubleshooting steps for common build issues
Would you like me to help draft the documentation updates?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)package-scripts.yml(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- CHANGELOG.md
🔇 Additional comments (1)
package-scripts.yml (1)
17-29: The prepack script implementation looks good but could be improved.
The script successfully addresses the Git dependency issue by ensuring the package is built during npm pack or npm publish. However, there are some potential improvements:
Consider these enhancements:
build:
prepack:
description: Build the project in the prepack script.
# 1. If the project is already built, do nothing;
# 2. Build the project but ignore TypeScript errors from missing devDependencies;
# 3. Check if the project is built now;
# 4. If it failed, print an error message (still follow https://docs.npmjs.com/cli/v8/using-npm/scripts#best-practices).
script: >
[ -f node_package/lib/ReactOnRails.js ] ||
- (nps build >/dev/null 2>&1 || true) &&
+ (nps build >/dev/null 2>build-error.log || (cat build-error.log && false)) &&
[ -f node_package/lib/ReactOnRails.js ] ||
- { echo 'Building react-on-rails seems to have failed!'; }
+ { echo 'Building react-on-rails failed! Please check build-error.log for details or run `nps build` manually.'; exit 1; }This change:
- Captures build errors in a log file for debugging
- Returns a non-zero exit code on failure
- Provides more helpful error message with debugging instructions
Let's verify the build process works correctly:
alexeyr-ci
force-pushed
the
alexeyr/prepack-script
branch
from
5b811d7 to
212e25b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
26-26: Verify the changelog entry matches the PR objectiveThe changelog entry "Enable use as a
git:dependency" aligns with the PR objective but could be more descriptive about the actual changes made to enable this functionality. Consider expanding the entry to mention that it fixes the package build issue after downloading.Suggested improvement:
- Enable use as a `git:` dependency. [PR 1664](https://github.com/shakacode/react_on_rails/pull/1664) by [alexeyr-ci](https://github.com/alexeyr-ci). + Enable use as a `git:` dependency by ensuring package is built after downloading. [PR 1664](https://github.com/shakacode/react_on_rails/pull/1664) by [alexeyr-ci](https://github.com/alexeyr-ci).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)package-scripts.yml(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- package-scripts.yml
alexeyr-ci
changed the title
Summary
When I tried to use a Git branch of the Node Renderer in Popmenu, it failed because the package isn't built after downloading. This PR fixes the issue. It should work for all package managers consuming this package:
prepareandprepack(and I think prepack is the more recommended) for Git dependencies;prepareprepackPull Request checklist
[ ] Add/update test to cover these changes[ ] Update documentationThis change is
Summary by CodeRabbit
New Features
Changes
Bug Fixes