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

feat: Allow annotation GrpcClient for target type 'annotation' #993

Merged

Conversation

313hemant313
Copy link
Contributor

Allow annotation GrpcClient for target type 'annotation'. #991

Copy link
Collaborator

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Please add a test case for this.

@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch from 8cc7a55 to c9fa66a Compare November 18, 2023 13:55
@313hemant313
Copy link
Contributor Author

@ST-DDT Test case added, please review.

@ST-DDT ST-DDT added the enhancement A feature request or improvement label Nov 18, 2023
@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch from 61e16d3 to f7fcc9e Compare November 18, 2023 14:34
@313hemant313 313hemant313 requested a review from ST-DDT November 18, 2023 14:43
@ST-DDT
Copy link
Collaborator

ST-DDT commented Nov 18, 2023

CI is red.

@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch from 6b29f73 to c60a73d Compare November 18, 2023 16:46
@313hemant313
Copy link
Contributor Author

Updated after ./gradlew :tests:spotlessApply. please run the job.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Nov 18, 2023

Please pull and run the ./gradlew spotlessApply command again.

@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch from 3db496d to 8a2ab2c Compare November 19, 2023 03:24
@313hemant313
Copy link
Contributor Author

./gradlew check and spotlessApply are success now.

@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch 2 times, most recently from 507416e to bd2be4b Compare November 19, 2023 09:07
Add testcases with usage of GrpcClientWrapper meta-annotation.
@313hemant313 313hemant313 force-pushed the target_annotation_grpcclient branch from bd2be4b to e321464 Compare November 19, 2023 09:10
@313hemant313
Copy link
Contributor Author

I was using java 19 version. changed it to 17 and pushed a new change with ./gradlew :tests:spotlessApply

Copy link
Collaborator

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for your contribution. ❤️

FFR: I find PRs easier to review if they aren't force pushed in between.

@313hemant313
Copy link
Contributor Author

313hemant313 commented Nov 19, 2023

FFR: I find PRs easier to review if they aren't force pushed in between.
Okay.

How to get this merged ?

@ST-DDT
Copy link
Collaborator

ST-DDT commented Nov 19, 2023

How to get this merged ?

You cannot merge this. We will have another reviewer look at this and then merge it.

@ST-DDT ST-DDT requested a review from yidongnan November 20, 2023 09:46
Copy link
Collaborator

@yidongnan yidongnan left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks for your contribution.

@yidongnan yidongnan added this to the 3.0.0 milestone Nov 20, 2023
@yidongnan yidongnan merged commit e6cb044 into grpc-ecosystem:master Nov 20, 2023
2 checks passed
@313hemant313 313hemant313 deleted the target_annotation_grpcclient branch November 20, 2023 14:34
@313hemant313
Copy link
Contributor Author

@ST-DDT / @yidongnan When will this change be in new maven release ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants