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

Meta annotations support #30

Closed
wants to merge 10 commits into from
Closed

Meta annotations support #30

wants to merge 10 commits into from

Conversation

BigMichi1
Copy link

@BigMichi1 BigMichi1 commented Mar 14, 2020

this is a pull request against 1.0.3 to support meta-annotations when trying to find the @ErrorHandling annotation on a class/interface or method

it would be really nice if this can be merged against 1.0.3 and a new 1.0.4 will be released because we can not upgrade to OpenFeign 10.x right now

this will fix #31

Copy link
Collaborator

@saintf saintf left a comment

Choose a reason for hiding this comment

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

Can you add a "how-to" or so on the readme - explaining what the usecase is and how to use it (I get how it works and reduces what would otherwise likely be boiler-plate code)?

Also - is the expectation that you use the meta annotations OR the explicit ones (but not both at the same time?) Not sure how they're meant to interact, but if they don't play well, we either need to make them play well with eachother (with test cases) or make sure that we are explicit in our How-to as well as add cases that force us not to mix-and-match (I picture when someone wants a generic version of handling a given set of standard errors - but a specific code on a specific class should generate something else and hence they add the meta-annotation plus one code on error handling... - what would then be the expected behaviour?

@BigMichi1
Copy link
Author

will improve it

@BigMichi1
Copy link
Author

@saintf i added some more test cases and also added the meta-annotations support to the README, would be nice if you can check again and let me know about further improvements

@BigMichi1
Copy link
Author

the PR is now also against master as it is easier to add this functionality

meta-annotations on meta-annotations are not supported
@BigMichi1
Copy link
Author

@saintf anything left here that i need to improve?

@BigMichi1
Copy link
Author

has been merged into the official repo OpenFeign/feign#1457

@BigMichi1 BigMichi1 closed this Jul 19, 2021
@BigMichi1 BigMichi1 deleted the meta-annotations-support branch July 19, 2021 07:17
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.

Meta-annotation support
2 participants