-
Notifications
You must be signed in to change notification settings - Fork 38
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
Adds short tutorial to readme and simple example, improves package docs #98
Conversation
* Adds simple example * Adds relevant package docs
49f3916
to
2cb5500
Compare
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.
Thanks for adding the godoc comments.
README.md
Outdated
Next we create the OpenPubkey client and call `opkClient.Auth`: | ||
```golang | ||
opkClient, err := client.New(op) | ||
pkt, err := opkClient.Auth(context.TODO()) |
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.
Should some comment be made about context.TODO()
?
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.
What do you suggest?
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.
What is it and why is it called TODO
? What is to be done?
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.
Changed this to use context.Background()
instead. I want to get developers up and running with requiring that make as few choices as possible. context.TODO()
implies that should replace the context with something else. Instead I'd rather give them a sane default.
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.
Should be fixed
dd97be0
to
64cd20e
Compare
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
This PR is written to address and close #78 by adding a short "Getting Started" tutorial to the readme.
To support this tutorial this PR also adds a very simple signing and verifying example and improves package docs on functions used in the tutorial.
This PR also cleans up most of the markdown warnings in the README and adds
typ
claim to the commitmentThis simple example will likely slowly replace the google example. Future work is to merge the Google example with the X509 example to create a general purpose signing utility. The creation this signing utility will be done in a future PR and will not be done in this PR.