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

Issue #5032 - Minimal Extensible Web App changes #5273

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Sep 15, 2020

Replacing PR #5033 as a minimal set of changes to allow for extensible Web Apps.

Either as WebAppContext or a ServletContextHandler.

Signed-off-by: Joakim Erdfelt [email protected]

@joakime joakime added Enhancement Sponsored This issue affects a user with a commercial support agreement labels Sep 15, 2020
@joakime joakime requested a review from gregw September 15, 2020 19:31
@joakime joakime self-assigned this Sep 15, 2020
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

just a few suggestions....

@joakime
Copy link
Contributor Author

joakime commented Sep 16, 2020

@gregw branch updated based on your review.
Not sure I like the BaseHolder.wrap() method, but the BaseHolder.unwrap() is fine.

@joakime
Copy link
Contributor Author

joakime commented Sep 16, 2020

@gregw I performed a partial revert of your changes in commit 31175e9 for the testcase.
Lets discuss this testcase, see if we can still have a simpler BaseHolder.wrap() method.

@joakime
Copy link
Contributor Author

joakime commented Sep 17, 2020

Once I get all clear from CI, I'll merge.

@joakime joakime force-pushed the jetty-9.4.x-5032-minimal-extensible-web-apps branch from d08831b to 59976dc Compare September 17, 2020 10:59
@joakime joakime merged commit 0db3663 into jetty-9.4.x Sep 17, 2020
@joakime joakime deleted the jetty-9.4.x-5032-minimal-extensible-web-apps branch September 17, 2020 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants