-
Notifications
You must be signed in to change notification settings - Fork 14
Add sub classing interfaces #372
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
base: main
Are you sure you want to change the base?
Add sub classing interfaces #372
Conversation
Signed-off-by: Fynn Juranek <[email protected]>
Signed-off-by: Fynn Juranek <[email protected]>
marcingrzejszczak
left a comment
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.
LGTM!
I wonder if that will work if the extension interface comes outside of our sources. I doubt it.
I start to wonder if we shouldn't think of putting more priority on creating some JSON like metadata that can later get merged - #130
| private final Collection<Class<?>> supportedInterfaces = new ArrayList<>( | ||
| Arrays.asList(SpanDocumentation.class, ObservationDocumentation.class)); |
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.
Doesn't this work?
| private final Collection<Class<?>> supportedInterfaces = new ArrayList<>( | |
| Arrays.asList(SpanDocumentation.class, ObservationDocumentation.class)); | |
| private final Collection<Class<?>> supportedInterfaces = Arrays.asList(SpanDocumentation.class, ObservationDocumentation.class); |
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.
This does not work, as Arrays.asList creates an immutable list, but I'm adding the found extending interfaces to the supportedInterfaces array. See AbstractSearchingFileVisitor.java#L81.
But maybe there is a better way of doing this, which I didn't think of?
| private final Collection<Class<?>> supportedInterfaces = new ArrayList<>( | ||
| Arrays.asList(MeterDocumentation.class, ObservationDocumentation.class)); |
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.
Doesn't this work?
| private final Collection<Class<?>> supportedInterfaces = new ArrayList<>( | |
| Arrays.asList(MeterDocumentation.class, ObservationDocumentation.class)); | |
| private final Collection<Class<?>> supportedInterfaces = Arrays.asList(MeterDocumentation.class, ObservationDocumentation.class); |
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.
This does not work, as Arrays.asList creates an immutable list, but I'm adding the found extending interfaces to the supportedInterfaces array. See AbstractSearchingFileVisitor.java#L81.
But maybe there is a better way of doing this, which I didn't think of?
This implementation enables sub-classing of the documentation interfaces, in order to get them recognized by the plugin as described in #338.
I've added some tests to ensure it behaves as expected, let me know if I should add some more cases.
This closes gh-338