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

Add the Thrift tutorial #4865

Merged
merged 76 commits into from
Jun 21, 2023
Merged

Add the Thrift tutorial #4865

merged 76 commits into from
Jun 21, 2023

Conversation

haneepark
Copy link
Contributor

Motivation:

  • Provide a tutorial for beginners who wants to use Apache Thrift with Armeria

Modifications:

  • Add a tutorial for writing an Apache Thrift service with Armeria

Result:

  • Adds the tutorial in the "THRIFT SERVICE" menu in armeria.dev site.

Please note:

  • This PR is a draft, since I need to address some questions before requesting the review. Please take a look at the comments starts with // TW: in index.mdx.

geun-son and others added 13 commits May 8, 2023 10:48
Add thrift tutorial images

Update site/src/pages/tutorials/thrift/blog/create-server.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/create-server.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/create-server.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-create.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-create.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-delete.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-update.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-create.mdx

Co-authored-by: Hanee Park <[email protected]>

Update implement-create.mdx

Update site/src/pages/tutorials/thrift/blog/implement-delete.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-delete.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-delete.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-read.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-read.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-read.mdx

Co-authored-by: Hanee Park <[email protected]>

Update site/src/pages/tutorials/thrift/blog/implement-read.mdx

Co-authored-by: Hanee Park <[email protected]>

Update index.mdx

Update site/src/pages/tutorials/thrift/blog/run-service.mdx

Co-authored-by: Hanee Park <[email protected]>

Update index.mdx

Update run-service.mdx

Update create-server.mdx

Update run-service.mdx

Rename run-service.mdx to create-client.mdx

Update toc.json

Update create-server.mdx

Update implement-read.mdx

Update implement-read.mdx

Update implement-read.mdx

Update implement-update.mdx

Update implement-delete.mdx

Update add-docservice.mdx

Update create-server.mdx

Update implement-create.mdx
@CLAassistant
Copy link

CLAassistant commented May 8, 2023

CLA assistant check
All committers have signed the CLA.

@jrhee17 jrhee17 added this to the 1.24.0 milestone May 8, 2023
@jrhee17
Copy link
Contributor

jrhee17 commented May 8, 2023

Looking forward to this PR!

This PR is a draft, since I need to address some questions before requesting the review. Please take a look at the comments starts with // TW: in index.mdx.

Will do 🙇

site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Looking good! Let me know when this PR is ready for review 🙇

site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-delete.mdx Outdated Show resolved Hide resolved
@haneepark
Copy link
Contributor Author

@jrhee17 @ikhoon @trustin @minwoox
The PR is ready for your review (_ _)

@haneepark haneepark marked this pull request as ready for review May 16, 2023 01:37
@haneepark
Copy link
Contributor Author

haneepark commented Jun 2, 2023

Perhaps I should have asked this earlier, but I'm wondering if there is a style guide or coding convention I can refer to when working on the Armeria project.

To be more specific,

  • I wonder if there is a maximum character limit for a single line. (Add the Thrift tutorial #4865 (comment))
  • I'm a bit confused about the indent rule for the method chaining, and I'm concerned that I might be inconsistent.

@jrhee17
Copy link
Contributor

jrhee17 commented Jun 5, 2023

Sorry about the confusion, I don't think there are a set of rules that we strictly follow 😅

I wonder if there is a maximum character limit for a single line.

In this project I think we have a rule for max 112 characters per line for java files

<property name="max" value="112"/>
.
This isn't enforced for other file types, but I guess this is a good rule to follow.

I'm a bit confused about the indent rule for the method chaining, and I'm concerned that I might be inconsistent.

I think the general rule is as long as the code block doesn't overflow we should be good.
For now, what do you think of sticking to always 2 space indentations?

I apologize since I think this comment may have caused confusion (actually I think I wasn't sure myself 😅 )

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Only nits, but nothing too serious. Still looks good to me 👍

@minwoox @ikhoon I think this PR is ready for review 😄

site/src/pages/tutorials/thrift/blog/index.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/run-server.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/run-server.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-read.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-read.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-update.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-delete.mdx Outdated Show resolved Hide resolved
site/src/pages/tutorials/thrift/blog/implement-delete.mdx Outdated Show resolved Hide resolved
@jrhee17 jrhee17 modified the milestones: 1.24.0, 1.25.0 Jun 9, 2023
@jrhee17
Copy link
Contributor

jrhee17 commented Jun 14, 2023

Reviews are a little slow at the moment since we're busy releasing the next version 😅 Sorry about it.
Will try to ping other reviewers once release is done (probably this week)

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Way to go, @haneepark! 🚀🙇

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @haneepark! ❤️🚀 This is a well-written tutorial!

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks for writing this awesome tutorial, @haneepark! 👍 🙇 🙇‍♂️

@ikhoon ikhoon modified the milestones: 1.25.0, 1.24.1 Jun 21, 2023
@ikhoon ikhoon merged commit a575329 into line:main Jun 21, 2023
@haneepark haneepark deleted the thrift-tutorial branch January 11, 2024 02:15
jrhee17 added a commit that referenced this pull request Apr 4, 2024
Motivation:
- There are differences among Armeria tutorials: structural difference
in introduction, style differences in writing.
- Different ways(curl, logging, testing) are used to verify its
implementation in each tutorial.
- The Thrift tutorial is most recently updated
(#4865) and using test code seems to
be the most appropriate way to go.

Modifications:
- Update introductions for each tutorials
- Update REST tutorial to use test code
- Rewrite gRPC tutorial to align with Thrift tutorial

Result:
- Updates tutorials on Armeria site

---------

Co-authored-by: jrhee17 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants