michael-o commented on code in PR #19:
URL: https://github.com/apache/velocity-tools/pull/19#discussion_r1683359688
##########
pom.xml:
##########
@@ -45,7 +45,9 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <slf4j.version>1.7.36</slf4j.version>
+ <slf4j.version>2.0.13</slf4j.version>
Review Comment:
This isn't required.
##########
velocity-tools-view-jsp/src/main/java/org/apache/velocity/tools/view/jsp/jspimpl/VelocityPageContext.java:
##########
@@ -409,20 +408,6 @@ public JspWriter getOut() {
return jspWriter;
}
- @SuppressWarnings("deprecation")
- @Override
- public javax.servlet.jsp.el.ExpressionEvaluator getExpressionEvaluator() {
- // Really, who cares?
- throw new UnsupportedOperationException("This class works only with JSP
2.1");
- }
-
- @SuppressWarnings("deprecation")
- @Override
- public javax.servlet.jsp.el.VariableResolver getVariableResolver() {
- // Really, who cares?
- throw new UnsupportedOperationException("This class works only with JSP
2.1");
- }
-
Review Comment:
Can you figure out frim with JSP API version this is not required anymore?
##########
velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ServletUtils.java:
##########
@@ -80,9 +80,9 @@ public static String getPath(HttpServletRequest request)
// will return the original (wrong) URI requested. The following
special
// attribute holds the correct path. See section 8.3 of the Servlet
// 2.3 specification.
- String path =
(String)request.getAttribute("javax.servlet.include.servlet_path");
+ String path =
(String)request.getAttribute("jakarta.servlet.include.servlet_path");
// also take into account the PathInfo stated on SRV.4.4 Request Path
Elements
- String info =
(String)request.getAttribute("javax.servlet.include.path_info");
+ String info =
(String)request.getAttribute("jakarta.servlet.include.path_info");
Review Comment:
same here
##########
velocity-tools-view/src/test/java/org/apache/velocity/tools/view/VelocityViewTest.java:
##########
@@ -80,8 +80,8 @@ public void
testGetTemplateHttpServletRequestHttpServletResponse() throws Resour
expect(config.getInitParameter(VelocityView.TOOLS_KEY)).andAnswer(eval(null));
expect(servletContext.getAttribute(ServletUtils.CONFIGURATION_KEY)).andAnswer(eval((String)null));
expect(servletContext.getResource(VelocityView.USER_TOOLS_PATH)).andAnswer(eval(null));
-
expect(request.getAttribute("javax.servlet.include.servlet_path")).andAnswer(eval("/charset-test.vm"));
-
expect(request.getAttribute("javax.servlet.include.path_info")).andAnswer(eval((String)null));
+
expect(request.getAttribute("jakarta.servlet.include.servlet_path")).andAnswer(eval("/charset-test.vm"));
+
expect(request.getAttribute("jakarta.servlet.include.path_info")).andAnswer(eval((String)null));
Review Comment:
Same here
##########
velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ServletUtils.java:
##########
@@ -80,9 +80,9 @@ public static String getPath(HttpServletRequest request)
// will return the original (wrong) URI requested. The following
special
// attribute holds the correct path. See section 8.3 of the Servlet
// 2.3 specification.
- String path =
(String)request.getAttribute("javax.servlet.include.servlet_path");
+ String path =
(String)request.getAttribute("jakarta.servlet.include.servlet_path");
Review Comment:
There is
https://tomcat.apache.org/tomcat-9.0-doc/servletapi/javax/servlet/RequestDispatcher.html#INCLUDE_SERVLET_PATH.
We should prefer over literals.
##########
pom.xml:
##########
@@ -158,19 +160,19 @@
<version>${slf4j.version}</version>
</dependency>
<dependency>
- <groupId>javax.servlet</groupId>
- <artifactId>javax.servlet-api</artifactId>
- <version>3.1.0</version>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ <version>6.0.0</version>
</dependency>
<dependency>
- <groupId>javax.servlet.jsp</groupId>
- <artifactId>javax.servlet.jsp-api</artifactId>
- <version>2.3.3</version>
+ <groupId>jakarta.servlet.jsp</groupId>
+ <artifactId>jakarta.servlet.jsp-api</artifactId>
+ <version>4.0.0</version>
</dependency>
<dependency>
- <groupId>javax.el</groupId>
- <artifactId>javax.el-api</artifactId>
- <version>3.0.0</version>
+ <groupId>jakarta.el</groupId>
+ <artifactId>jakarta.el-api</artifactId>
+ <version>6.0.0</version>
Review Comment:
Wrong also, see https://tomcat.apache.org/whichversion.html Tomcat 10
baseline.
##########
velocity-tools-examples/velocity-tools-examples-showcase/src/main/webapp/WEB-INF/web.xml:
##########
@@ -19,10 +19,13 @@
under the License.
-->
-<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
- xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd";
- version="3.1">
- <!-- servlets -->
+<web-app
+ xmlns="http://xmlns.jcp.org/xml/ns/javaee";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
http://xmlns.jcp.org/xml/ns/javaee/web-app_4_0.xsd";
+ version="4.0"
+>
Review Comment:
Note: This is technically not required, they will run just fine on a new
container...
##########
velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ServletUtils.java:
##########
@@ -80,9 +80,9 @@ public static String getPath(HttpServletRequest request)
// will return the original (wrong) URI requested. The following
special
// attribute holds the correct path. See section 8.3 of the Servlet
// 2.3 specification.
- String path =
(String)request.getAttribute("javax.servlet.include.servlet_path");
+ String path =
(String)request.getAttribute("jakarta.servlet.include.servlet_path");
// also take into account the PathInfo stated on SRV.4.4 Request Path
Elements
Review Comment:
These comments seem to outdated. We should either make them version-neutral
or remove altogether.
##########
pom.xml:
##########
@@ -45,7 +45,9 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
- <slf4j.version>1.7.36</slf4j.version>
+ <slf4j.version>2.0.13</slf4j.version>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
This is wrong. Look into the parent (up or velocity master), there is a
single property for the version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
