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

Update README.md #152

Merged
merged 2 commits into from
Feb 2, 2020
Merged

Update README.md #152

merged 2 commits into from
Feb 2, 2020

Conversation

rhcarvalho
Copy link
Contributor

Clarify usage of sentry.Flush with a simple and realistic code example, plus some miscellaneous changes.

Related to #106.

Copy link
Contributor Author

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

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

Annotated changes 📖


## Usage

By default, Sentry Go SDK uses asynchronous transport, which in the code example below requires an explicit awaiting for event delivery to be finished using `sentry.Flush` method. It is necessary, because otherwise the program would not wait for the async HTTP calls to return a response, and exit the process immediately when it reached the end of the `main` function. It would not be required inside a running goroutine or if you would use `HTTPSyncTransport`, which you can read about in `Transports` section.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I try to clarify when and why Flush needs to be called.

> before terminating the program, arrange for `sentry.Flush` to be called before
> the program terminates.

Example:

```go
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The example now:

  • Can be built with a Go compiler;
  • Does something realistic;
  • Has comments teasing out details (maybe something else needs to be called out?);
  • Avoids the sentry.CaptureException followed by sentry.Flush anti-pattern. Typically, sentry.Flush shall only be called once during termination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bruno-garcia could I get your 👀 onto this example, please? Might be easier to read it in https://github.com/getsentry/sentry-go/blob/aac7497bdcff2cdb5d069ee6ae5aeb0aee49e014/README.md#usage


## Community

Join Sentry's [`#go` channel on Discord](https://discord.gg/Ww9hbqr) to get involved and help us improve the SDK!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only rewrapped.

@@ -103,8 +173,11 @@ For more detailed information about how to get the most out of `sentry-go` there

## License

Licensed under the BSD license, see `LICENSE`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added links.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@@ -0,0 +1,156 @@
package main
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the old example/basic/main.go unchanged.

@@ -1,156 +1,53 @@
// This is an example program that makes an HTTP request and prints response
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fresh new code for a "basic" example. The same code is used in the repo's README.md file.

bruno-garcia
bruno-garcia previously approved these changes Jan 29, 2020
README.md Outdated Show resolved Hide resolved
Sentry SDK for Go automation moved this from In progress to Reviewer approved Jan 29, 2020
Sentry SDK for Go automation moved this from Reviewer approved to Review in progress Jan 29, 2020
README.md Outdated
Comment on lines 121 to 122
// Either set your DSN here or set the SENTRY_DSN environment variable.
Dsn: "",
// Enable printing of SDK debug messages. Useful when getting started or
// trying to figure something out.
Debug: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bruno-garcia this is my second take. Good Developer eXperience for you?

@rhcarvalho
Copy link
Contributor Author

@kamilogorek I'd like your input here, specially the updated example that goes into the front-facing README. Thanks!

@getsentry getsentry deleted a comment from rhcarvalho Jan 30, 2020
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
example/basic/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

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

Just a few nitpicks

Move existing "basic" example into new directory.
Main points:

- Rewrite Go version requirements to avoid constant manual maintenance:
we don't need to list out version numbers explicitly.

- Add links where they might be helpful.

- Clarify configuration and usage sections. Use a simple example that
people can copy from as a starting point.
@rhcarvalho
Copy link
Contributor Author

Let's get this in as an incremental improvement.

@rhcarvalho rhcarvalho merged commit 5893ba1 into getsentry:master Feb 2, 2020
Sentry SDK for Go automation moved this from Review in progress to Done Feb 2, 2020
@rhcarvalho rhcarvalho deleted the readme branch February 2, 2020 20:33
rhcarvalho added a commit to rhcarvalho/sentry-docs that referenced this pull request Feb 2, 2020
rhcarvalho added a commit to getsentry/sentry-docs that referenced this pull request Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants