-
Notifications
You must be signed in to change notification settings - Fork 161
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
Be aware of skinny war workaround in OSGi support #8111
Conversation
// don't use onStartup method because a fake servlet context is | ||
// passed here: no need to detect classloaders in OSGi case | ||
((ClassLoaderAwareServletContainerInitializer) initializer).process( | ||
filterClasses(handleTypes.orElse(null)), | ||
getOsgiServletContext()); | ||
} catch (ServletException e) { | ||
throw new RuntimeException( |
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.
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.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)
a discussion (no related file):
This is not the whole story.
2.2
doesn't support OSGi for many various reasons
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.
Reviewed 1 of 1 files at r1.
Dismissed @vaadin-bot from a discussion.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)
a discussion (no related file):
Previously, denis-anisimov (Denis) wrote…
This is not the whole story.
2.2
doesn't support OSGi for many various reasons
Seems like using
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient-osgi</artifactId>
<version>4.5.12</version>
</dependency>
Poses no problems for the downloader and can be used as is.
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.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @caalador)
a discussion (no related file):
Previously, caalador wrote…
Seems like using
<dependency> <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient-osgi</artifactId> <version>4.5.12</version> </dependency>
Poses no problems for the downloader and can be used as is.
Unfortunately it's not enough.
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.
ready for review
Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained
SonarQube analysis reported 6 issues Watch the comments in this conversation to review them. 5 extra issuesNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
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.
Reviewed 5 of 5 files at r2.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained
Was not tested in
2.2
because OSGi is completely broken (#8109).Fixes #8105
This change is