-
Notifications
You must be signed in to change notification settings - Fork 229
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 tracing hello world #270
Add tracing hello world #270
Conversation
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.
Looks great! Just a few minor nits
java/tracing-hello-world/README.md
Outdated
|
||
## Before you begin | ||
|
||
This sample assumes you have [Java 8][java8] installed. |
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.
might be good to emphasis that a jre is not enough and a jdk is required
|
||
Generate a credentials file by running the [application-default login](https://cloud.google.com/sdk/gcloud/reference/auth/application-default/login) command: | ||
|
||
gcloud auth application-default login |
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.
I think we should discourage people from using application default creds and encourage them to use service accounts
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.
I created issue #271 to switch all examples to service accounts. For now, can we keep application-default credentials so that we can keep this issue consistent with the others?
java/tracing-hello-world/pom.xml
Outdated
@@ -0,0 +1,116 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<!-- | |||
~ Copyright (c) 2016 Google Inc. All Rights Reserved. |
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.
please update the year
import java.io.IOException; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
import javax.swing.plaf.synth.SynthSeparatorUI; |
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.
please clean up imports
The comments were addressed. PTAL. |
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
* Adding a Hello World Java with tracing. * Updating HelloWorld docs and readm * Addressing comments
* Add tracing hello world (#270) * Adding a Hello World Java with tracing. * Updating HelloWorld docs and readm * Addressing comments * Some fixes to the readme * Adding new lines
I added a hello world with tracing example.
You can see the readme here: https://github.com/sduskis/cloud-bigtable-examples/tree/add_tracing_hello_world/java/tracing-hello-world