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

initial cut of CLI generation #349

Merged
merged 18 commits into from
Mar 28, 2023
Merged

initial cut of CLI generation #349

merged 18 commits into from
Mar 28, 2023

Conversation

ahl
Copy link
Collaborator

@ahl ahl commented Mar 3, 2023

This is a prototype feature, not ready to be released, but I'd rather have it in the repo than maintaining it in a branch. It shouldn't impact consumers that don't explicitly use it.

Closes oxidecomputer/oxide.rs#46

cc: @david-crespo, @zephraph, @karencfv

Copy link
Contributor

@karencfv karencfv left a comment

Choose a reason for hiding this comment

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

woohoo!!!! 🎉 Thanks for working on this!

}
});

// TODO parameter for body as input json (--body-input?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we anticipate customers requiring JSON input in the first iteration of the CLI? If not maybe we can punt this for later if it's needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect we need it in the first iteration of the CLI at least as much as the customer would.

Copy link
Contributor

Choose a reason for hiding this comment

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

it'd be necessary for the $ oxide api command, not so much for the others imo

});

// TODO parameter for body as input json (--body-input?)
// TODO parameter to output a body template (--body-template?)
Copy link
Contributor

Choose a reason for hiding this comment

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

By body template do you mean a table?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to have a parameter that outputs "blank" JSON output so that the user can output that to a file, edit it to fill in appropriate values, and then input it to another option that accepts a JSON-formatted file as input.

) -> Result<String> {
let output = self.cli(spec, crate_name)?;

let content = rustfmt_wrapper::rustfmt_config(
Copy link
Contributor

Choose a reason for hiding this comment

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

according to #362 , wouldnt rustfmt not be used in the -impl ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point; we'll work this out.

@ahl ahl marked this pull request as ready for review March 28, 2023 21:33
@ahl ahl merged commit 10dc4ca into main Mar 28, 2023
@ahl ahl deleted the cli branch March 28, 2023 21:54
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.

get the progenitor cli branch merged
4 participants