Skip to content
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.

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

feat(cargo-shuttle): v0.48.2, colored platform outputs, fixes #1886

Merged
merged 9 commits into from
Oct 10, 2024

Conversation

jonaro00
Copy link
Member

No description provided.

@jonaro00 jonaro00 changed the title fix(cargo-shuttle): beta fixes feat(cargo-shuttle): colored platform outputs Oct 2, 2024
@jonaro00 jonaro00 force-pushed the beta-final-fixes branch 2 times, most recently from 643f683 to f35ce48 Compare October 3, 2024 13:13
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request introduces colored platform outputs and refines the CLI interface for the cargo-shuttle command-line tool. Key changes include:

  • Added color-coded platform information in lib.rs for improved user experience
  • Updated command descriptions and visibilities in args.rs to prepare features for wider use
  • Removed 'BETA' labels from several commands, indicating stability
  • Adjusted the 'Deploy' command to use 'WIP' instead of 'BETA' for Docker image deployment

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Comment on lines +184 to +187
if self.beta {
eprintln!("{}", "INFO: Using NEW platform API (shuttle.dev)".green());
} else {
eprintln!("{}", "INFO: Using OLD platform API (shuttle.rs)".blue());
Copy link

Choose a reason for hiding this comment

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

style: Consider using a constant for the color strings (e.g., 'green' and 'blue') to improve maintainability

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Based on the provided information and the previous review, here's a concise summary of the additional changes and potential areas of focus for this pull request:

  1. Updated project configuration structure in config.rs:

    • Deprecated the assets field
    • Introduced new deploy and build configurations
    • Updated the include method to handle both new and deprecated asset configurations
  2. Minor version bump in Cargo.toml:

    • Updated cargo-shuttle version from 0.48.0 to 0.48.1
  3. Refined error handling in lib.rs:

    • Improved error messages related to platform-specific commands

These changes aim to enhance the deployment and build processes, providing more granular control over file inclusion and asset management. The review should focus on the implementation of the new configuration structure and its potential impact on existing projects.

4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings

Comment on lines +184 to +187
if self.beta {
eprintln!("{}", "INFO: Using NEW platform API (shuttle.dev)".green());
} else {
eprintln!("{}", "INFO: Using OLD platform API (shuttle.rs)".blue());
Copy link

Choose a reason for hiding this comment

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

style: Consider using constants for the color strings (e.g., 'green' and 'blue') to improve maintainability

Comment on lines +191 to +194
eprintln!(
"{}",
format!("INFO: Targeting non-default API: {url}").yellow(),
);
Copy link

Choose a reason for hiding this comment

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

style: Use a constant for the 'INFO:' prefix to ensure consistency across messages

@@ -1118,7 +1124,7 @@ impl Shuttle {

async fn logs_beta(&self, args: LogsArgs) -> Result<()> {
if args.follow {
eprintln!("Streamed logs are not yet supported on beta.");
eprintln!("Streamed logs are not yet supported on the new platform.");
Copy link

Choose a reason for hiding this comment

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

style: Update error message to use 'NEW platform' for consistency with other messages

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR introduces colored output for platform-related messages in the cargo-shuttle CLI and updates API key handling. Here's a concise summary of the key changes:

  • Added colored output in cargo-shuttle/src/lib.rs for better visual distinction between platform versions and messages
  • Simplified API key handling in cargo-shuttle/src/config.rs by removing the ApiKey type
  • Introduced SHUTTLE_LOGIN_URL_BETA constant in common/src/constants.rs for beta platform support

These changes aim to improve user experience and prepare for beta platform support. The review should focus on the implementation of these features and their impact on usability.

3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings

Comment on lines +184 to +187
if self.beta {
eprintln!("{}", "INFO: Using NEW platform API (shuttle.dev)".green());
} else {
eprintln!("{}", "INFO: Using OLD platform API (shuttle.rs)".blue());
Copy link

Choose a reason for hiding this comment

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

style: Use constants for the color strings (e.g., 'green' and 'blue') to improve maintainability

Comment on lines +191 to +194
eprintln!(
"{}",
format!("INFO: Targeting non-default API: {url}").yellow(),
);
Copy link

Choose a reason for hiding this comment

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

style: Use a constant for the 'INFO:' prefix to ensure consistency across messages

@@ -1118,7 +1124,7 @@ impl Shuttle {

async fn logs_beta(&self, args: LogsArgs) -> Result<()> {
if args.follow {
eprintln!("Streamed logs are not yet supported on beta.");
eprintln!("Streamed logs are not yet supported on the new platform.");
Copy link

Choose a reason for hiding this comment

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

style: Update error message to use 'NEW platform' for consistency with other messages

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR introduces significant changes to the Shuttle CLI, focusing on simplifying the command structure and improving user experience. Here's a concise summary of the key updates:

  • Changed CLI command structure from 'cargo shuttle' to 'shuttle' across multiple files
  • Simplified deployment process by removing the separate project creation step
  • Updated documentation and error messages to reflect new command structure
  • Introduced colored output for platform-specific messages in cargo-shuttle/src/lib.rs

These changes aim to streamline the user experience and prepare for the new beta platform. The implementation appears consistent across various files, including documentation and error messages.

12 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This pull request makes a minor version update to the cargo-shuttle package, incrementing it from version 0.48.1 to 0.48.2. Here's a concise summary of the key changes:

  • Updated cargo-shuttle version to 0.48.2 in cargo-shuttle/Cargo.toml
  • Introduced colored output for platform-specific messages in the CLI

This update likely includes small improvements or bug fixes to enhance the user experience with the Shuttle CLI tool.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@jonaro00 jonaro00 changed the title feat(cargo-shuttle): colored platform outputs feat(cargo-shuttle): colored platform outputs + fixes Oct 9, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This pull request makes minor improvements to the cargo-shuttle CLI tool, focusing on enhancing user experience and clarity. Here's a concise summary of the key changes:

  • Updated project deletion confirmation message in cargo-shuttle/src/lib.rs:
    • Now uses project ID instead of project name for more precise identification
    • Improves clarity in the warning message for users

The changes are minimal but contribute to a better user experience when managing projects through the Shuttle CLI.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@jonaro00 jonaro00 changed the title feat(cargo-shuttle): colored platform outputs + fixes feat(cargo-shuttle): v0.48.2, colored platform outputs, fixes Oct 9, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This pull request introduces a small but important change to the runtime/src/start.rs file, enhancing the user experience for the Shuttle CLI tool. Here's a concise summary of the key modification:

  • Conditional logging in runtime/src/start.rs:
    • Provides different documentation URLs based on the beta argument
    • Directs users to the appropriate documentation for their platform (beta or non-beta)

This change improves user guidance by ensuring they are referred to the correct documentation resources, tailored to their specific platform environment.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@jonaro00 jonaro00 merged commit be7b731 into main Oct 10, 2024
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant