-
Notifications
You must be signed in to change notification settings - Fork 784
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
polishing FeignClientsRegistrar #1038
polishing FeignClientsRegistrar #1038
Conversation
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.
Hello @birariro, I'm not sure why you've proposed this change. We do need to use beanFactory
to properly resolve the attributes.
Hello @OlgaMaciaszek
And |
Keep the for testing comment |
@birariro It's helpful to know that the visibility level has been raised or sth has been added for testing purposes. That's why we put these comments there, but actually in this case, if we remove the other overloaded method, it should be |
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.
Thanks @birariro. Looks good. Can you please just add your full name and surname with @author
tag to the javadoc of the class and update the date in the license comment to 2013-2024
?
@OlgaMaciaszek I have applied the changes as requested. |
getName (Map<String, Object>)
does not need to be for test purposes.