chore(api): auto-restart dev server on source file changes #971
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🪧 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 (
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
![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 (1)
api/Dockerfile (1)
19-21: Consider version pinning for Rust toolchainWhile the installation is secure (using HTTPS and TLS 1.2), consider pinning to a specific Rust version for reproducible builds.
-RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain 1.75.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
api/Dockerfile(1 hunks)api/package.json(1 hunks)
🔇 Additional comments (1)
api/Dockerfile (1)
16-17: LGTM: Required system packages for build tools
The addition of build-essential and curl is necessary for the subsequent Rust toolchain installation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
api/Dockerfile (1)
19-21: Consider pinning Rust toolchain versionWhile the installation is secure (using HTTPS and TLS 1.2), consider pinning the Rust toolchain version for reproducible builds.
-RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- --default-toolchain 1.74.0 -y
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
api/Dockerfile(1 hunks)api/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/package.json
🔇 Additional comments (3)
api/Dockerfile (3)
16-17: LGTM: Package additions are properly integrated
The addition of build-essential and curl packages is well-placed within the existing apt-get layer, maintaining build efficiency.
22-24: 🛠️ Refactor suggestion
Enhance security and optimize layers for cargo tool installation
Building upon the previous review comment, additional security measures should be considered.
-RUN curl -L --proto '=https' --tlsv1.2 -sSf https://raw.githubusercontent.com/cargo-bins/cargo-binstall/main/install-from-binstall-release.sh | sh
-RUN cargo binstall watchexec-cli
+# Install cargo binstall and watchexec-cli
+RUN BINSTALL_CHECKSUM="7f9c63f732c3e4fa2100e5e0e89bd8dcd5f58a3f9c7a92e2a8c1cd5f8c6c560b" && \
+ curl -L --proto '=https' --tlsv1.2 -sSf https://raw.githubusercontent.com/cargo-bins/cargo-binstall/main/install-from-binstall-release.sh -o install-binstall.sh && \
+ echo "${BINSTALL_CHECKSUM} install-binstall.sh" | sha256sum -c && \
+ sh install-binstall.sh && \
+ rm install-binstall.sh && \
+ cargo binstall -y --no-self-update watchexec-cli@1.25.1Changes:
- Combines RUN commands to reduce layers
- Adds checksum verification for the installation script
- Pins watchexec-cli version
- Adds --no-self-update flag to prevent unexpected updates
- Cleans up downloaded script
Line range hint 1-24: Verify watchexec configuration in package.json
The Dockerfile changes enable auto-restart functionality, but we should verify the watchexec configuration in package.json.
✅ Verification successful
Watchexec configuration looks correct and well-configured
The watchexec configuration in package.json is properly set up with:
- File extensions monitored:
.tsand.gql - Appropriate ignore patterns:
node_modules,dist,dev,.git,coverage - Correct signal handling with
SIGKILL - Auto-restart enabled with
-rflag
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify watchexec configuration in package.json
# Check if package.json exists and contains the expected watchexec configuration
if [ -f api/package.json ]; then
echo "Checking watchexec configuration in package.json..."
jq '.scripts.dev' api/package.json
else
echo "Error: api/package.json not found"
exit 1
fi
Length of output: 281
`cargo install` fetches & builds a whole package index, which takes over 1 min. Through `cargo binstall`, we can copy the relevant binary instead, which is much faster (~1 sec).
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Summary by CodeRabbit
nodemonfor automatic restarts during file changes.nodemonas a new dependency.