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

Steve/better docs #110

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Steve/better docs #110

merged 1 commit into from
Apr 30, 2020

Conversation

stevesoundcloud
Copy link
Contributor

No description provided.

README.md Outdated Show resolved Hide resolved
docs/404.html Outdated
@@ -0,0 +1,24 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It believe it's generated by default with jekyll new. Seems ok to me.

docs/index.md Outdated
**Json:**

```scala
val client = new HaberdasherClientJson(httpService)
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't we want to remove the jsonclient? let's do that and not document it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's part of the Twirp protocol, and also an officially supported proto3 serialization format, so we do want to support it as a general matter in this library.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we had a discussion about this already and decided to remove it. Happy to discuss this again if needed.

Copy link
Member

Choose a reason for hiding this comment

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

The latest twinagle release fixes the issue with the Json client, so we don't need to remove it. Let's mention the binary protobuf client first, and documenting that it should be used unless a human being needs to look at the data on the wire?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

docs/index.md Outdated
```scala
import twitch.twirp.example.haberdasher.HaberdasherService

class ServiceImpl extends HaberdasherService {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call this HaberdasherServiceImpl then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

@ccmtaylor ccmtaylor merged commit 5f3845b into master Apr 30, 2020
@ccmtaylor ccmtaylor deleted the steve/better-docs branch April 30, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants