-
Notifications
You must be signed in to change notification settings - Fork 105
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
[JBTM-3176] stop using unnamed package as io.smallrye.metrics fails #255
[JBTM-3176] stop using unnamed package as io.smallrye.metrics fails #255
Conversation
Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana-quickstart/152/ |
final String ManifestMF = "Manifest-Version: 1.0\n" | ||
+ "Dependencies: org.jboss.modules,org.jboss.msc,org.jboss.jts\n"; | ||
+ "Dependencies: org.jboss.jts\n"; |
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.
Can I just check, is this a tidy up bit or is it required for the fix? Happy to have it but if it is not required for JBTM-3176 please can you separate it to a different commit (no specific issue number required as it is in the quickstarts but please say something like "Clean-up during JBTM-3176" so we can track it but separate to the core fix)
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.
Yes, it's only tidy up the dependencies. When I start investigating the quickstart it seemed to me strange that there is dependency defined to org.jboss.modules
which I think is pretty rare to be defined. Thus I removed that for tidying things up.
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.
OK, so please do split this into a separate commit and please do raise the WFLY - thanks again!
I approved but please see my comment. If it is required as part of the JBTM-3176 that is interesting to know, but else just to separate it. I think we should raise a WFLY because although not good practice smallrye has to tolerate no package code I expect. |
Sorry, I saw your comment on smallrye from the Jira. I think we still need the WFLY though to ensure it takes the upgrade |
fe02d03
to
e6a3590
Compare
@tomjenkinson I updated the PR with splitting the changes to two commits. The WFLY issue is at https://issues.jboss.org/browse/WFLY-12403 |
Thanks @ochaloup great diagnosis! |
https://issues.jboss.org/browse/JBTM-3176