Skip to content
This repository was archived by the owner on Jan 28, 2026. It is now read-only.

fix: retrieve to stdout failure#31

Merged
dtbuchholz merged 1 commit intomainfrom
dtb/fix-stdout-retrieval
Jan 8, 2024
Merged

fix: retrieve to stdout failure#31
dtbuchholz merged 1 commit intomainfrom
dtb/fix-stdout-retrieval

Conversation

@dtbuchholz
Copy link
Contributor

Summary

Fixes a bug where vaults retrieve -o - ... fails.

Details

The retrieve command with -o - should write to stdout. However, this was not working due to a small operator typo in which the tmpFile variable was not being assigned a value, so the write to stdout wasn't executing because tmpFile was seen as nil. This was due to the usage of := over = within a conditional block scope.

How it was tested

Build & then run vaults retrieve --output - bafybeigpvzdvjgde5a6ayj7265a6ns3i4mvhhr6pqtlebpil5imkeh4ncy | car extract to pipe the output to car extract, which unpacks the car file from stdout successfully.

The `retrieve` command with `-o -` should write to stdout. However, this was not working due to a small typo in which the `tmpFile` variable was not being assigned a value, so the stdout wasnt executing. This was due to the usage of `:=` over `=` within a conditional block scope.
@dtbuchholz dtbuchholz requested a review from brunocalza January 8, 2024 04:09
@dtbuchholz dtbuchholz added the bug Something isn't working label Jan 8, 2024
@dtbuchholz
Copy link
Contributor Author

dtbuchholz commented Jan 8, 2024

@brunocalza not sure how i missed this...it was originally working when i wrote that code, but i must've accidentally changed the assignment operator before this got merged. just double checking—do we have tests set up? if so, i could make sure there's one for this command flag. (i'm not as familiar with go so didn't want to mess with anything.)

@brunocalza
Copy link
Contributor

@brunocalza not sure how i missed this...it was originally working when i wrote that code, but i must've accidentally changed the assignment operator before this got merged. just double checking—do we have tests set up? if so, i could make sure there's one for this command flag. (i'm not as familiar with go so didn't want to mess with anything.)

This repo is lacking in tests, because it's hard to implement tests for CLI :( but we can think of a way for sure, just didn't want to spend too much time on that since the directions of the project wasn't clear

@dtbuchholz dtbuchholz merged commit 3de5c82 into main Jan 8, 2024
@dtbuchholz dtbuchholz deleted the dtb/fix-stdout-retrieval branch January 8, 2024 17:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants