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

add support for passing in input from STDIN #72

Merged
merged 1 commit into from
Mar 30, 2017

Conversation

concaf
Copy link
Collaborator

@concaf concaf commented Mar 13, 2017

This commit lets passing input from STDIN to opencompose like in
the following example -

cat example.yaml | opencompose -f -

An error will be returned if reading from STDIN fails.

Fixes #63

ping @tnozicka @surajssd @kadel

Copy link
Contributor

@tnozicka tnozicka left a comment

Choose a reason for hiding this comment

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

small nit

@@ -62,12 +63,22 @@ func GetValidatedObject(v *viper.Viper, cmd *cobra.Command, out, outerr io.Write
}

var ocObjects []*object.OpenCompose
var data []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

please move the declaration back to the loop where it was before. extending the scope is error prone

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@surajssd
Copy link
Contributor

When I run

$ opencompose convert -f -

It remains stuck, is this behavior okay?

@concaf
Copy link
Collaborator Author

concaf commented Mar 16, 2017

@surajssd yep, this is the desired behavior, it is waiting for input.
Paste a valid yaml and press Ctrl-D, and it will proceed.

return nil, fmt.Errorf("unable to read file '%s': %s", file, err)
// Check if the passed resource points to STDIN or not
if file == "-" {
data, err = ioutil.ReadAll(os.Stdin)
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious: does cobra provide a way to read from STDIN ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@surajssd I didn't find any method in cobra regarding this, do you know something that can help?

@surajssd
Copy link
Contributor

How do we test this?

@kadel
Copy link
Member

kadel commented Mar 16, 2017

How do we test this?

ioutil.ReadAll requires parameter that implements io.Reader interface.
You can use anything that has io.Reader interface to fake io.Stdin (for example temporary file).

But, I don't think that we should worry too much about testing this few lines.
I know controversial idea 😆 , but I'm willing to own it 🤣

This commit lets passing input from STDIN to opencompose like in
the following example -

`cat example.yaml | opencompose -f -`

An error will be returned if reading from STDIN fails.

Fixes redhat-developer#63
@concaf
Copy link
Collaborator Author

concaf commented Mar 16, 2017

@surajssd we could test this, but we need to add tests for GetValidatedObject as well as the entire convert.go, so imo it would be better to test these in another PR.
WDYT?
CC: @kadel @tnozicka

@kadel kadel dismissed tnozicka’s stale review March 30, 2017 06:58

'small nit' was fixed. and code looks ok

@kadel kadel merged commit 885542a into redhat-developer:master Mar 30, 2017
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.

4 participants