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

fix: AnnotationHandler should read annotations from parent class #4985

Conversation

MartinUhlen
Copy link
Contributor

@MartinUhlen MartinUhlen commented Apr 24, 2024

Fixes #4983 which is a regression from #4506

AnnotationHandler should be able to read annotations from parent class.

The fix is really simple (too simple? why was it there in the first place?), just removing a redundant line in AnnotationHandler.

The new test case and all existing test cases passes.

@shakuzen
Copy link
Member

The fix is really simple (too simple? why was it there in the first place?), just removing a redundant line in AnnotationHandler.

I'm not sure, but it looks like it may have been a mistake. I don't see the line in the original pull request #3727, but I see it in the follow-up commit bc59c47. I'm not sure where it came from.

@shakuzen shakuzen changed the base branch from main to 1.11.x April 24, 2024 07:45
@shakuzen shakuzen force-pushed the AnnotationHandler_cannot_see_methods_from_parent_class branch from 80d102f to fb3a535 Compare April 24, 2024 07:46
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

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

Thank you for the quick turnaround on fixing this. I've rebased on the 1.11.x branch so we can merge forward and include it in all affected versions.

@shakuzen
Copy link
Member

@bpernetupgrade would you mind taking a look? Does the test capture the scenario you had in mind? Any other tests you think we should add?

@shakuzen shakuzen merged commit ff16ad1 into micrometer-metrics:1.11.x Apr 24, 2024
6 checks passed
@bpernetupgrade
Copy link

@bpernetupgrade would you mind taking a look? Does the test capture the scenario you had in mind? Any other tests you think we should add?

@shakuzen I am not super familiar with AspectJ, but yeah the test scenario is correct and matched the situation I encountered. Thanks.

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.

AnnotationHandler can't see methods from parent class
3 participants