Add dry run before letting user go in tui config#717
Add dry run before letting user go in tui config#717brayan-trejo wants to merge 4 commits intomainfrom
Conversation
brayan-trejo
requested review from
Dabnsky,
Ferelith-maker,
foodprocessor and
jfantinhardesty
as code owners
| var outBuf, errBuf bytes.Buffer | ||
| cmd.Stdout = &outBuf | ||
| cmd.Stderr = &errBuf | ||
| err := cmd.Run() | ||
| if err != nil { | ||
| return fmt.Errorf("Dry run failed: %v\n%s", err, errBuf.String()) | ||
| } |
There was a problem hiding this comment.
Do you want to capture stdout too?
Maybe you could simplify this and use CombinedOutput instead of Run().
There was a problem hiding this comment.
stdout is captured but I don't do anything with it for now because at least with the current implementation of --dry-run nothing is printed to stdout when successful, but I figured it's still good practice to capture it.
There was a problem hiding this comment.
I looked into it a bit more. With CombinedOutput(), a combined byte slice is returned and you cannot reliably tell where stdout ends and where stderr begins. The intent here is to cleanly separate these two, and print stderr only when an error occurs. Even though stdout doesn't print anything useful now, in the future if that changes, it'd be nice to have this template to conditionally do something with stdout.
Signed-off-by: Brayan Trejo <117423453+brayan-trejo@users.noreply.github.com>
3 participants
What type of Pull Request is this? (check all applicable)
Describe your changes in brief
**cloudfuse mount <test-mnt-point> --config-file <config-file> --dry-run --passphrase <passphrase>**at end of tui configuration to ensure the generated configuration config.yaml.aes file functions correctly.Checklist
Related Issues