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

docs: add initialization of LogEntry instance in the v2 example #46

Merged
merged 3 commits into from Jul 1, 2020

Conversation

ymotongpoo
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #44

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 29, 2020
@ymotongpoo ymotongpoo changed the title Add initialization of LogEntry instance in the v2 example docs: add initialization of LogEntry instance in the v2 example Jun 29, 2020
Copy link
Contributor Author

@ymotongpoo ymotongpoo left a comment

Choose a reason for hiding this comment

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

Add questions on the contents of the v2 example.

@@ -84,7 +84,9 @@ Using the API
from google.cloud import logging_v2

client = logging_v2.LoggingServiceV2Client()
entries = []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though this PR only changed the issue that the list entries is empty, I think we need to add how to set resources and log_name. What do you think?

Choose a reason for hiding this comment

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

Yes, that would be useful to have too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I added the note.

@simonz130 simonz130 self-requested a review June 29, 2020 17:26
README.rst Outdated
@@ -84,7 +84,9 @@ Using the API
from google.cloud import logging_v2

client = logging_v2.LoggingServiceV2Client()
entries = []
e = logging_v2.types.LogEntry(
text_payload="text")

Choose a reason for hiding this comment

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

please update "Text" to something more meaningful, like "this is a log statement"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the exact phrase :)

@ymotongpoo
Copy link
Contributor Author

@simonz130 Thank you Simon for the check. I updated the pull request.

@simonz130 simonz130 added api: logging Issues related to the googleapis/python-logging API. priority: p2 Moderately-important priority. Fix may not be included in next release. kokoro:run Add this label to force Kokoro to re-run the tests. automerge Merge the pull request once unit tests and other checks pass. labels Jul 1, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 1, 2020
@simonz130 simonz130 added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 1, 2020
@gcf-merge-on-green gcf-merge-on-green bot merged commit 251ac93 into googleapis:master Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/python-logging API. automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement. kokoro:run Add this label to force Kokoro to re-run the tests. priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getting started sample shows how to iterate over empty logs list
4 participants