Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Include trigger type in trigger options help header #2256

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Jan 29, 2024

Fixes #2252.

Note this does not deduplicate common options - it only adds the trigger type as part of the section header.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@@ -255,6 +255,12 @@ fn warn_if_wasm_build_slothful() -> sloth::SlothGuard {
sloth::warn_if_slothful(SLOTH_WARNING_DELAY_MILLIS, format!("{message}\n"))
}

fn help_heading<E: TriggerExecutor>() -> Option<&'static str> {
let heading = format!("{} TRIGGER OPTIONS", E::TRIGGER_TYPE.to_uppercase());
let as_str = Box::new(heading).leak();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using Box::leak is fine here IMO, but I think perhaps using a static and OnceCell is another, arguably less hacky, solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed; leak is perfectly fine for this purpose.

@itowlson itowlson merged commit 96498fe into fermyon:main Jan 29, 2024
11 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spin up --help on multi trigger not clear
4 participants