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

Make AnnotatedService public #5628

Merged
merged 26 commits into from
May 7, 2024
Merged

Conversation

chickenchickenlove
Copy link
Contributor

@chickenchickenlove chickenchickenlove commented Apr 22, 2024

Motivation:

  • In the past, AnnotatedService provided specialized information for annotated services such as Method, service instance, and the default status, and so on. however, users could not access this information because it was declared with a package-private access modifier, so it was not part of the public API.

Modifications:

  • Split AnnotatedService into two parts: AnnotatedService (interface) and DefaultAnnotatedService (implementation).
  • AnnotatedService : The methods that can be used by users through the Public API are declared in the AnnotatedService interface.
  • DefaultAnnotatedService: Methods intended for internal use are declared in DefaultAnnotatedService.

Result:

@trustin trustin added new feature sprint Issues for OSS Sprint participants labels Apr 23, 2024
chickenchickenlove and others added 15 commits April 28, 2024 19:29
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/DefaultAnnotatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…onfigSetters.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…onfigSetters.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
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.

Almost there - Let's go!

chickenchickenlove and others added 3 commits April 30, 2024 19:30
…onfigSetters.java

Co-authored-by: Trustin Lee <t@motd.kr>
…erviceNamingUtil.java

Co-authored-by: Trustin Lee <t@motd.kr>
@ikhoon ikhoon added this to the 1.29.0 milestone May 2, 2024
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.

Nice work! Thanks, @chickenchickenlove. 🙇‍♂️🙇‍♂️

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.

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.

Pushed a small commit with minor cleanups. Thanks @chickenchickenlove ! 🙇 👍 🙇

Copy link
Member

@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.

Looks really great! Thanks, @chickenchickenlove! 👍 👍 👍

@trustin trustin merged commit 7d562ce into line:main May 7, 2024
14 of 15 checks passed
@trustin
Copy link
Member

trustin commented May 7, 2024

Please make sure the PR description is up to date and has no grammar errors or incorrect class names next time. Also, please do not mix up motivation, modification and result. For example, what you initially wrote at the motivation section is the result of your work.

@chickenchickenlove
Copy link
Contributor Author

Thanks for your comment.
I updated the PR description following your guide. 🙇‍♂️

Dogacel pushed a commit to Dogacel/armeria that referenced this pull request Jun 8, 2024
Motivation:

- Related issue: line#5382
- It'd be nice if a user can access the information provided by
`AnnotatredService`, but it is not part of the public API.

### Modifications:

- Split `AnnotatedService` into `AnnotatedService` (interface) and 
  `DefaultAnnotatedService` (implementation).

### Result:
- Closes line#5382

Before:
```java
        ServiceRequestContext serviceRequestContext = ServiceRequestContext.current();
        AnnotatedService service = serviceRequestContext.config().service().as(AnnotatedService.class);

        // This doesn't compile.
        // service.method(); 
        // service.defaultStatus();
```

After:
```java
        ServiceRequestContext serviceRequestContext = ServiceRequestContext.current();
        AnnotatedService service = serviceRequestContext.config().service().as(AnnotatedService.class);

        // This compiles and provides useful properties.
        service.method(); 
        service.defaultStatus();
```

---------

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
Co-authored-by: Trustin Lee <t@motd.kr>
Co-authored-by: jrhee17 <guins_j@guins.org>
Co-authored-by: minux <songmw725@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature sprint Issues for OSS Sprint participants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make AnnotatedService public
5 participants