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

[fud2] add flag to print commands instead of running them #2259

Merged
merged 3 commits into from
Oct 7, 2024
Merged

[fud2] add flag to print commands instead of running them #2259

merged 3 commits into from
Oct 7, 2024
+14 −3

Conversation

jku20
Copy link
Collaborator

@jku20 jku20 commented Aug 9, 2024

It does what it says on the tin.

@jku20 jku20 added the C: fud2 experimental driver label Aug 9, 2024
sampsyo
sampsyo requested changes Aug 11, 2024
View reviewed changes
Copy link
Contributor

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

Seems like a very cool thing to add… but what do you think about making this a "mode" for tidiness's sake?

fud2/fud-core/src/cli.rs Outdated Show resolved Hide resolved
@jku20 jku20 requested a review from sampsyo August 12, 2024 20:00
@jku20
Copy link
Collaborator Author

jku20 commented Aug 12, 2024

In addition to what you said, implementation-wise definitely nicer to do this as a "mode." Having to feed the flag through a Run was kind of awkward.

@jku20 jku20 requested review from EclecticGriffin and removed request for sampsyo October 7, 2024 02:23
EclecticGriffin
EclecticGriffin approved these changes Oct 7, 2024
View reviewed changes
Copy link
Collaborator

@EclecticGriffin EclecticGriffin left a comment

Choose a reason for hiding this comment

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

LGTM!

@jku20 jku20 merged commit eb398ea into main Oct 7, 2024
18 checks passed
@jku20 jku20 deleted the fud2-ninja-print-run-commands branch October 7, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sampsyo sampsyo sampsyo requested changes

@EclecticGriffin EclecticGriffin EclecticGriffin approved these changes

Assignees
No one assigned
Labels
C: fud2 experimental driver
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jku20 @sampsyo @EclecticGriffin