Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Implement @Attribute Injection. #5547
Implement @Attribute Injection. #5547
Changes from 1 commit
eac6431
2801cdd
889d887
94a93af
42ef9a2
ab8a08a
3fac84e
f3f8b74
821e18f
dce9520
fc5771a
f017563
1927e46
ee7a466
65868b7
1d0b6c7
1877117
bdb0088
29543f9
957378f
2893d3d
c70c2fb
c1de466
e6632fa
065e43a
e80075f
5874745
c6519ab
bb087a7
4560943
0dd3e77
65bc886
c27629b
3943f70
3df348b
7230534
72d26b2
8f3631c
9013388
588c8bb
70d76cb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 51 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java#L51
Check warning on line 53 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java#L53
Check warning on line 56 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java#L56
Check warning on line 58 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedElementNameUtil.java#L58
Check warning on line 454 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L453-L454
Check warning on line 645 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L639-L645
Check warning on line 648 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L647-L648
Check warning on line 651 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L651
Check warning on line 810 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L810
Check warning on line 813 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L812-L813
Check warning on line 815 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L815
Check warning on line 817 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L817
Check warning on line 819 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L819
Check warning on line 831 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L823-L831
Check warning on line 836 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L833-L836
Check warning on line 839 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L839
Check warning on line 843 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L843
Check warning on line 847 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L847
Check warning on line 849 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L849
Check warning on line 851 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L851
Check warning on line 859 in core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java
Codecov / codecov/patch
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java#L856-L859
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.
Because the value is required anyways, it's probably better to enforce this at compile-time rather than handling it as an error later.
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.
I agree. i committed your suggestion!
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.
Do you think that inferring the name of
AttributeKey
fromParameter.getName()
is not a good practice?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.
@ikhoon nim, thanks for your comment. 🙇♂️
I don't think so. I believe each has its own strengths.
I tend to prefer explicit aspects.
However, I am ready to adapt to the preferences of the framework developer, as well!
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.
I talked about this with other maintainers and they thought the naming convention of
AttributeKey
is different from method parameter names.Let's keep it as is.
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.
I see! thanks a lot 🙇♂️