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.

Sign up for GitHub

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

Jump to bottom

Remove the unused code from the MethodInfo #1785

Merged
merged 2 commits into from Oct 11, 2022
Merged

Remove the unused code from the MethodInfo #1785

merged 2 commits into from Oct 11, 2022

Conversation

wplong11
Copy link
Collaborator

@wplong11 wplong11 commented Oct 9, 2022

The MethodInfo.configKey field is not more used since PR #1757 removing AsyncInvocation

@wplong11
Remove the unused code from the MethodInfo
d3210c1 
The MethodInfo.configKey field is not more used since PR OpenFeign#1757 removing AsyncInvocation
kdavisk6
kdavisk6 requested changes Oct 9, 2022
View reviewed changes
Copy link
Member

@kdavisk6 kdavisk6 left a comment

Choose a reason for hiding this comment

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

configKey is required for use with HystrixFeign. This is not unused code.

@wplong11
Copy link
Collaborator Author

wplong11 commented Oct 9, 2022 •
edited

@kdavisk6

configKey is required for use with HystrixFeign. This is not unused code.

MethodInfo.configKey is not used.
HystrixFeign uses configKey, but it uses Feign.configKey, not MethodInfo.configKey.
I deleted configKey from MethodInfo, and HystrixFeign does not reference MethodInfo

@kdavisk6 kdavisk6 added the ready to merge Will be merged if no other member ask for changes label Oct 10, 2022
@velo velo merged commit 6378ae2 into OpenFeign:master Oct 11, 2022
@wplong11 wplong11 deleted the remove-unused-code-from-MethodInfo branch October 11, 2022 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdavisk6 kdavisk6 kdavisk6 approved these changes

Assignees
No one assigned
Labels
ready to merge Will be merged if no other member ask for changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wplong11 @kdavisk6 @velo