feat: impl live reloading for local runs #1991
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements live reloading for local development using the bacon crate, allowing automatic application restarts when files change.
- Added a new
--bacon/-bflag toshuttle runin/cargo-shuttle/src/args.rsthat enables watch mode - Created
/cargo-shuttle/src/bacon.rsto handle bacon integration, including version checking (requires 3.8.0+) and configuration - Modified
/cargo-shuttle/src/lib.rsto conditionally use bacon when the flag is enabled - Updated integration tests to include the new flag parameter (set to false by default)
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR simplifies the bacon integration for live reloading in local development environments.
- Removed redundant
default_value = "false"from thebaconflag incargo-shuttle/src/args.rsas boolean flags are false by default in clap - Improved error messaging in
cargo-shuttle/src/bacon.rswhen bacon is not installed or has an incompatible version - Added clear configuration for bacon to restart the application when files change
Greptile AI
2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR finalizes the bacon integration for live reloading with a more robust implementation.
- Added a constant
BACON_CONFIGincargo-shuttle/src/bacon.rswith proper configuration for killing and restarting processes - Improved error handling in
check_bacon()with more descriptive error messages when bacon is not installed - Enhanced version parsing logic to extract version number from bacon's output
Greptile AI
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| let version_str = stdout.split_whitespace().nth(1).ok_or_else(|| { | ||
| anyhow::anyhow!("Failed to parse bacon version: unexpected output format") | ||
| })?; |
There was a problem hiding this comment.
style: The version parsing logic assumes a specific output format from the bacon command. Consider adding a more robust parsing approach that can handle different output formats or version strings.
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR completes the implementation of live reloading for local runs by adding platform-specific configurations.
- Split bacon configuration into platform-specific files with
/cargo-shuttle/src/bacon.unix.tomlfor Unix and/cargo-shuttle/src/bacon.windows.tomlfor Windows - Added conditional compilation in
/cargo-shuttle/src/bacon.rsto select the appropriate configuration file based on platform - Implemented proper process termination with
pkill -TERM -Pfor Unix andtaskkill /F /T /PIDfor Windows
Greptile AI
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR adds newline characters to the bacon configuration files for better compatibility with text editors and tools.
- Added newline at the end of
cargo-shuttle/src/bacon.unix.tomlfor Unix systems - Added newline at the end of
cargo-shuttle/src/bacon.windows.tomlfor Windows systems
Greptile AI
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
|
@jonaro00 can you please review this? |
|
Thanks for working on this. I've been swamped recently. |
| .await? | ||
| .success() | ||
| .then_some(()) | ||
| .ok_or_else(|| anyhow::anyhow!("bacon process failed"))?; |
There was a problem hiding this comment.
Also requires importing anyhow::Context trait.
| .ok_or_else(|| anyhow::anyhow!("bacon process failed"))?; | |
| .context("bacon process failed")?; |
| let version_str = stdout.split_whitespace().nth(1).ok_or_else(|| { | ||
| anyhow::anyhow!("Failed to parse bacon version: unexpected output format") | ||
| })?; |
There was a problem hiding this comment.
| let version_str = stdout.split_whitespace().nth(1).ok_or_else(|| { | |
| anyhow::anyhow!("Failed to parse bacon version: unexpected output format") | |
| })?; | |
| let version_str = stdout.split_whitespace().nth(1).context("Failed to parse bacon version: unexpected output format")?; |
| .arg("--version") | ||
| .output() | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("Failed to execute bacon: {}\nPlease ensure bacon is installed ('cargo install bacon') and you have the necessary permissions", e))?; |
There was a problem hiding this comment.
You can also use context here to add context about the error, and the underlying error will be displayed as a cause, example:
Error: Please ensure bacon is installed ('cargo install bacon') and you have the necessary permissions
Caused by:
No such file or directory (os error 2)
| .map_err(|e| anyhow::anyhow!("Failed to execute bacon: {}\nPlease ensure bacon is installed ('cargo install bacon') and you have the necessary permissions", e))?; | |
| .context("Failed to execute bacon\nPlease ensure bacon is installed ('cargo install bacon') and you have the necessary permissions")?; |
fixes #1967