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

Fix spin up --help omitting many options if app is not ready to lock #934

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Dec 5, 2022

Partially fixes #932.

There remains an outstanding issue where spin up --help will skip "trigger" arguments if the app contains no components. This happens because the serialisation objects make component a mandatory field, so we cannot even load the file in this case. This could be addressed by making component optional, and validating that at least one component exists during the locking process (or otherwise after the initial load). This is probably a bit of a niche case though.

Signed-off-by: itowlson ivan.towlson@fermyon.com

@itowlson itowlson requested a review from lann December 5, 2022 00:14
Copy link
Collaborator

@lann lann left a comment

Choose a reason for hiding this comment

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

Minor style suggestions

Comment on lines 159 to 160
let help_args = vec![OsString::from("--help-args-only")];
cmd.args(help_args);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can simplify since it doesn't have to match the type of trigger_args:

Suggested change
let help_args = vec![OsString::from("--help-args-only")];
cmd.args(help_args);
cmd.arg("--help-args-only");

@@ -199,6 +188,25 @@ impl UpCommand {
}
}

fn build_locked_app(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe?

Suggested change
fn build_locked_app(
fn write_locked_app(

Comment on lines +1089 to +1096
name = "unbuilt"
trigger = { type = "http", base = "/" }
version = "0.1.0"
[[component]]
id = "unbuilt"
source = "DOES-NOT-EXIST.wasm"
[component.trigger]
route = "/..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

TOML doesn't mind indent...

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson force-pushed the up-help-when-app-not-ready-to-lock branch from 4502cc8 to 06d1c15 Compare December 5, 2022 19:22
@itowlson itowlson merged commit eec68c1 into fermyon:main Dec 5, 2022
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.

spin up --help doesn't display all options unless application is fully ready to load
2 participants