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

implement grpc request scope #198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bherrmann2
Copy link

@bherrmann2 bherrmann2 commented Apr 5, 2021

This is to port over the grpc request scope added in https://github.com/yidongnan/grpc-spring-boot-starter in the following PR https://github.com/yidongnan/grpc-spring-boot-starter/pull/259 (by @ST-DDT)

@Bean
@Scope(scopeName = GRpcRequestScope.GRPC_REQUEST_SCOPE_NAME, proxyMode = ScopedProxyMode.TARGET_CLASS)
BeanType beanName() {
        return new BeanType();
}

@bherrmann2
Copy link
Author

@jvmlet can you take a look at this?

@jvmlet
Copy link
Collaborator

jvmlet commented Apr 9, 2021

Did you ask the author of this feature if he is OK with copying the implementation as is?

@bherrmann2
Copy link
Author

The copyright notice in the code gives explicit permission to copy it, but I can attempt to reach out

@bherrmann2
Copy link
Author

I have reached out here: #177 (comment)

@ST-DDT
Copy link

ST-DDT commented Apr 11, 2021

I consent to my code being ported over.

Copy link

@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 (I haven't tested it though).

@bherrmann2
Copy link
Author

I have now tagged you in the PR description @ST-DDT . I wasn't able to before you approved/commented on the PR since you weren't a contributor on this project.

@ST-DDT
Copy link

ST-DDT commented Apr 11, 2021

FFR: It is possible even without that, but auto completion does not work.

@bherrmann2
Copy link
Author

@jvmlet I got the permission and approval. Let me know if this is ok

@bherrmann2
Copy link
Author

@jvmlet can you take another look?

@bherrmann2
Copy link
Author

@jvmlet Can you please review this? I would still like to get this in

@deft1991
Copy link

Can you please merge it?

@bherrmann2
Copy link
Author

@deft1991 . We need @jvmlet to reivew, approve, and merge.

@hehe717
Copy link

hehe717 commented Aug 25, 2021

@jvmlet is there any update?
I want to use this feature.

@hehe717 hehe717 mentioned this pull request Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants