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

feat: add version deps command [modversion] #6115

Merged
merged 3 commits into from
Apr 4, 2019
Merged

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Mar 21, 2019

  • Tests

@ghost ghost assigned Kubuxu Mar 21, 2019
@ghost ghost added the status/in-progress In progress label Mar 21, 2019
return nil
},
Encoders: cmds.EncoderMap{
cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, dep Dependency) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason, if I change Dependency to *Dependency here I get no output, no error and the encoder is not being run (tested with panic).

Copy link
Member

Choose a reason for hiding this comment

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

@Kubuxu
Copy link
Member Author

Kubuxu commented Mar 21, 2019

@Stebalien this requires us to bump to Go1.12, is that ok?

return nil
},
Encoders: cmds.EncoderMap{
cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, dep Dependency) error {
Copy link
Member

Choose a reason for hiding this comment

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

fmt.Fprintf(w, " => %s", dep.ReplacedBy)
}
fmt.Fprintf(w, "\n")
return errors.New("test")
Copy link
Member

Choose a reason for hiding this comment

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

foobar

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like this error gets ignored. Interesting.

Copy link
Member

Choose a reason for hiding this comment

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

diff --git a/core/commands/version.go b/core/commands/version.go
index 9c5c666d0..c9632f799 100644
--- a/core/commands/version.go
+++ b/core/commands/version.go
@@ -118,9 +118,13 @@ Print out all dependencies and their versions.`,
 			}
 			return
 		}
-		res.Emit(toDependency(&info.Main))
+		if err := res.Emit(toDependency(&info.Main)); err != nil {
+			return err
+		}
 		for _, dep := range info.Deps {
-			res.Emit(toDependency(dep))
+			if err := res.Emit(toDependency(dep)); err != nil {
+				return err
+			}
 		}
 		return nil
 	},

@Stebalien
Copy link
Member

@Kubuxu personally, I'm fine with a go 1.12 bump. Non-buggy go modules require that as well.


Other comment: needs test.

fmt.Fprintf(w, " => %s", dep.ReplacedBy)
}
fmt.Fprintf(w, "\n")
return errors.New("test")
Copy link
Member

Choose a reason for hiding this comment

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

diff --git a/core/commands/version.go b/core/commands/version.go
index 9c5c666d0..c9632f799 100644
--- a/core/commands/version.go
+++ b/core/commands/version.go
@@ -118,9 +118,13 @@ Print out all dependencies and their versions.`,
 			}
 			return
 		}
-		res.Emit(toDependency(&info.Main))
+		if err := res.Emit(toDependency(&info.Main)); err != nil {
+			return err
+		}
 		for _, dep := range info.Deps {
-			res.Emit(toDependency(dep))
+			if err := res.Emit(toDependency(dep)); err != nil {
+				return err
+			}
 		}
 		return nil
 	},

@Stebalien
Copy link
Member

@Kubuxu we'll also need to update the go version in CI.

@Kubuxu
Copy link
Member Author

Kubuxu commented Mar 22, 2019

I know, just didn't have time to handle it today.

@Kubuxu Kubuxu changed the title feat: add version deps command feat: add version deps command [modversion] Mar 27, 2019
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Copy link
Member Author

Kubuxu commented Apr 4, 2019

This is ready.

@Kubuxu
Copy link
Member Author

Kubuxu commented Apr 4, 2019

Wait with it, I've done something weird in sharness.

License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Copy link
Member Author

Kubuxu commented Apr 4, 2019

I think it is done.

@Kubuxu
Copy link
Member Author

Kubuxu commented Apr 4, 2019

It is ready.

@Stebalien Stebalien merged commit b50b8f4 into master Apr 4, 2019
@Stebalien Stebalien deleted the feat/modversion branch April 4, 2019 20:53
@ghost ghost removed the status/in-progress In progress label Apr 4, 2019
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.

2 participants