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

xsbt-web-plugin 4.2.4 -> sbt-war 5.0.0-M2 #131

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

earldouglas
Copy link

@earldouglas earldouglas commented Oct 9, 2024

We might want to wait for the non-milestone 5.0.0 release, in which case this PR can serve as a placeholder.

sbt-war 5 uses com.heroku:webapp-runner, which uses Tomcat (currently version 10.1.28, which targets the 6.0 servlet spec); would we want to use the Eclipse Jetty runner instead?

@earldouglas earldouglas marked this pull request as ready for review October 9, 2024 22:47
@@ -14,7 +14,7 @@
$ sbt new scalatra/scalatra.g8
$ cd <name-of-app>
$ sbt
> Jetty/start
Copy link
Author

Choose a reason for hiding this comment

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

This might require other documentation updates around the Scalatra ecosystem.

Copy link
Member

@takezoe takezoe Oct 10, 2024

Choose a reason for hiding this comment

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

Yeah, we will work on it once this PR is merged.

@takezoe
Copy link
Member

takezoe commented Oct 10, 2024

Thank you for raising this. Looks like we need to fix CI😅 Will work on it in another PR.

@takezoe
Copy link
Member

takezoe commented Oct 14, 2024

@earldouglas CI has been fixed in the master branch. Could you rebase this pull request on it?

would we want to use the Eclipse Jetty runner instead?

In theory, using Tomcat wouldn't be a problem as Scalatra is a pure servlet-based framework which doesn't rely on container specific features.

However, Jetty is used for many purposes in Scalatra, like integration test support, standalone deployment, etc. So, it would be better to use Jetty by default if possible.

@earldouglas earldouglas marked this pull request as draft October 14, 2024 15:33
@earldouglas
Copy link
Author

earldouglas commented Oct 14, 2024

CI has been fixed in the master branch

Thanks!

it would be better to use Jetty by default if possible

Gotcha. I'll experiment with using Jetty by default instead.

Marking this PR as draft until these are both addressed:

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.

2 participants