-
Notifications
You must be signed in to change notification settings - Fork 349
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 install SDK goal to make #458
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #458 +/- ##
=======================================
Coverage 91.59% 91.59%
=======================================
Files 64 64
Lines 3142 3142
=======================================
Hits 2878 2878
Misses 184 184
Partials 80 80 Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #458 +/- ##
=======================================
Coverage 91.59% 91.59%
=======================================
Files 64 64
Lines 3142 3142
=======================================
Hits 2878 2878
Misses 184 184
Partials 80 80 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please paste the output for the new targets?
It should be available in travis output. |
Travis sets the |
The output in travis is the same as locally. What are your concerns with output? Could you be more specific please? locally:
travis: https://travis-ci.org/jaegertracing/jaeger-operator/jobs/541164268#L230
|
Interesting, I thought Travis was more verbose. My concern is that all our targets are silent, and these new targets are verbose. Instead of showing the commands (curl, chmod, go get...), omit them and their outputs, unless a failure happens. |
I don't really care this is one-time command folks run when start with the project. Do you want to add |
Signed-off-by: Pavol Loffay <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need to print out something like "Installing SDK..." at the beginning of the task, like the other tasks in the Makefile
. Like:
@echo Installing SDK...
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay [email protected]