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

Add Extension parsing method to AnnotationScannerExtension #250

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

MikeEdgar
Copy link
Member

Fixes #245

  • Add tests to require that Jackson library is not necessary for
    annotation scanning when providing custom implementation of
    AnnotationScannerExtension#parseExtension
  • Refactor ExceptionMapper search to eliminate Sonar "bugs"
  • Replace TestNG @Test annotation with JUnit annotation in ExceptionMapperScanTests

- Add tests to require that Jackson library is not necessary for
annotation scanning when providing custom implementation of
`AnnotationScannerExtension#parseExtension`
@MikeEdgar MikeEdgar force-pushed the 245_extension_reader branch from db7742f to b912b1d Compare January 31, 2020 01:52
Copy link
Contributor

@EricWittmann EricWittmann left a comment

Choose a reason for hiding this comment

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

LGTM

implementation/pom.xml Show resolved Hide resolved
* @param key the name of the extension property
* @param value the string value of the extension
*/
default Object parseExtension(String key, String value) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@tjquinno, please let me know if this method definition will cover what you need to parse extensions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @MikeEdgar . I think the answer is "yes." I had to shift gears to some other things briefly so I'm not in a position right now to try this (probably first thing next week). But I think it looks as if it'll work.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MikeEdgar Just FYI, I'm using a local build of the latest SmallRye master (with this PR) and I see my AnnotationScannerExtension parseExtension method being invoked as expected. Thanks again.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tjquinno - very good, glad to hear that. I expect there will be a release in the next few days.

@MikeEdgar MikeEdgar merged commit 60a9ece into smallrye:master Jan 31, 2020
@MikeEdgar MikeEdgar deleted the 245_extension_reader branch February 2, 2020 12:22
@tjquinno
Copy link
Contributor

tjquinno commented Feb 4, 2020

@MikeEdgar Appreciate the very fast turnaround on this. Is there somewhere I can look to see about the plans and timing for the next release (which would presumably include this and other changes)? Thanks.

@MikeEdgar
Copy link
Member Author

@tjquinno - minor releases are typically as needed.

@kenfinnigan, @EricWittmann, @msavy - I think we are at a good point to tag and release. Any objections?

@msavy
Copy link
Contributor

msavy commented Feb 4, 2020 via email

@EricWittmann
Copy link
Contributor

No objections.

@MikeEdgar
Copy link
Member Author

@kenfinnigan - please tag/release smallrye-open-api when you get a chance. Thanks

@kenfinnigan
Copy link
Contributor

As we're using jakarta dependencies now, does it require a version bump to 1.3.x?

@MikeEdgar
Copy link
Member Author

I am OK with bumping the minor version, but I think it should be 1.2.x. Does it matter if the major/minor differs from the upstream MP version?

@kenfinnigan
Copy link
Contributor

Sorry, I did mean 1.2.x.

I saw 1.1.20 and somehow read 1.2.x instead

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.

Decouple OpenApiAnnotationScanner from Jackson
5 participants