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.

Sign up for GitHub

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

Jump to bottom

rust cli #185

Merged
merged 2 commits into from
Dec 21, 2024
Merged

rust cli #185

merged 2 commits into from
Dec 21, 2024

Conversation

burgerdev
Copy link
Collaborator

First impression of a concise Rust implementation of the CLI - turned out I had this sitting on an abandoned branch.

@burgerdev burgerdev requested a review from pkern October 11, 2024 21:01
@pkern
Copy link
Member

pkern commented Oct 21, 2024

Generally looks good to me. I have nothing to add to your TODOs. Thanks! :)

burgerdev
burgerdev commented Oct 30, 2024
View reviewed changes
Copy link
Collaborator Author

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

Thanks for the review! I'll leave a couple more TODOs :)

rust/Cargo.toml Show resolved Hide resolved
rust/src/cli/bin.rs Show resolved Hide resolved
rust/src/cli/bin.rs Outdated Show resolved Hide resolved
@burgerdev burgerdev changed the title wip: rust cli rust cli Dec 14, 2024
@burgerdev burgerdev marked this pull request as ready for review December 14, 2024 17:44
pkern
pkern requested changes Dec 15, 2024
View reviewed changes
Copy link
Member

@pkern pkern left a comment

Choose a reason for hiding this comment

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

Only one typo and a question about errors... Thank you very much, otherwise LGTM

rust/src/cli/bin.rs Outdated Show resolved Hide resolved
rust/src/cli/bin.rs Outdated Show resolved Hide resolved
@burgerdev burgerdev added this to the v0.2.0 milestone Dec 21, 2024
This was referenced Dec 21, 2024
Open
Open
pkern
pkern approved these changes Dec 21, 2024
View reviewed changes
Copy link
Member

@pkern pkern left a comment

Choose a reason for hiding this comment

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

LGTM.

It'd be good to also have a presubmit GitHub Action, but let's leave that for a followup PR.

@pkern pkern merged commit dc6eb8a into google:master Dec 21, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkern pkern pkern approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@burgerdev @pkern