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

More complete examples, better READMEs #13

Merged
merged 9 commits into from
Oct 3, 2023
Merged

More complete examples, better READMEs #13

merged 9 commits into from
Oct 3, 2023

Conversation

swi-jared
Copy link
Contributor

@swi-jared swi-jared commented Oct 2, 2023

  • Added a database example in the HTTP example
  • Created a gRPC client/server example
  • Updated and created READMEs
  • Updated a bunch of dependencies

Here's a link to the branch in github so you can click around the READMEs.

@swi-jared swi-jared self-assigned this Oct 2, 2023
@swi-jared swi-jared added documentation Improvements or additions to documentation and removed work in progress labels Oct 2, 2023
@swi-jared swi-jared marked this pull request as ready for review October 2, 2023 23:16
@swi-jared swi-jared requested a review from a team October 2, 2023 23:16
examples/README.md Outdated Show resolved Hide resolved

go 1.20

// TODO don't use the local repo
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO for the user playing with this example right? Then this is ok here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no. It's super not important given that these are examples, but yes it's also a hint to the user "don't do this".

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this nice example now has the added responsibility of showing a bit of context in db queries, could we also have some of that info added to this readme? Some of that is already commented in main.go, e.g. The SQL commenter helps associate queries with traces. and It's important to inject the request context into the query call because it carries the current trace information

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've updated the intro paragraph. Care to take another look?

@tammy-baylis-swi
Copy link
Contributor

Thanks @swi-jared ! Just a few suggestions/questions I had.

Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

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

Thank you @swi-jared !!

@swi-jared swi-jared merged commit 8a0a6ca into main Oct 3, 2023
@swi-jared swi-jared deleted the readme-updates branch October 3, 2023 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Development

Successfully merging this pull request may close these issues.

2 participants