Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2323428808 > > Here are a few takeaways for you why my approach with a single module will _not_ work for Velocity Tools: > > ``` > > * It violates Maven principals to have one main artifact per module, two different namespaces are two main artifacts > > > > * How to solve different sets of dependencies at any scope? You can't. > > > > * How to build classifier artifacts of classier artifacts? JAvadoc/sources? You can't. > > > > * Downstream consumers doing analysis of dep trees? You can't. > > ``` > > > > > > > > > > > > > > > > > > > > > > > > There are more arguments I cannot remember ATM, but we have here only one choice: Yet another module in the reactor which just reuses the sources somehow, rewrites them and builds appropriately. > > What I wouldn't also do is to duplicate the sample/showcase modules at all. Why? If a developer cannot make the mental stretch to replace some namespace then he shouldn't deal with it at all. I'd solely focus on Servlet/JSP related code: `velocity-tools-view-jsp` and `velocity-tools-view` (which actually should rather be `velocity-tools-view-servlet`). > > I will support any solution that results in a way to use `velocity-tools-view` in a modern version of Tomcat (10+). If you are going to split that project, your newly recommended approach would result (e.g.) in the following four artifacts: > > * velocity-tools-view-servlet (javax) > > * velocity-tools-view-jsp (jaxax) > > * velocity-tools-view-servlet-jakarta (jakarta) > > * velocity-tools-view-jsp-jakarta (jakarta) > > > I would be totally fine with that. This sounds like more than a simple pull request. You probably don't want me trying to change the module names and creating new ones. I'd be willing to try, but I'll stand by for your recommendation. First is to create the new jakarta modules. I wouldn't even copy the source code, I would use the `generate-sources` phase to copy the on the fly and the `process-sources` will rewrite the servlet package. > > If a developer cannot make the mental stretch to replace some namespace then he shouldn't deal with it at all. > > While I agree that it would be unnecessary to duplicate the sample/showcase modules, we were all beginner developers at some point. As a courtesy, perhaps you could simply add a comment above the relevant imports to suggest that the examples will also work with the jakarta version of the modules if "javax." is replaced with "jakarta.". At least that would show that it is intentional and prevent any unnecessary questions. IIRC, Freemarker also included some comments like that somewhere. I'd leave this afterwards, but I would do is that I would skip the deployment of all showcases and examples being for them being in Maven Central has zero value. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
nbubna commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2323071475 I agree with Claude. We should ask people moving to new versions with old javax to update their module name. The jakarta versions should be the new default going forward. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
arkanovicz commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2323022193 A new module seems inevitable, a copy and replace of the sources seems appropriate. But I'd rather have the main artifacts pointing to jakarta and the new ones named -javax, as at some point we'll drop the later. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2322948423 > Here are a few takeaways for you why my approach with a single module will _not_ work for Velocity Tools: > > * It violates Maven principals to have one main artifact per module, two different namespaces are two main artifacts > > * How to solve different sets of dependencies at any scope? You can't. > > * How to build classifier artifacts of classier artifacts? JAvadoc/sources? You can't. > > * Downstream consumers doing analysis of dep trees? You can't. > > > There are more arguments I cannot remember ATM, but we have here only one choice: Yet another module in the reactor which just reuses the sources somehow, rewrites them and builds appropriately. > > What I wouldn't also do is to duplicate the sample/showcase modules at all. Why? If a developer cannot make the mental stretch to replace some namespace then he shouldn't deal with it at all. I'd solely focus on Servlet/JSP related code: `velocity-tools-view-jsp` and `velocity-tools-view` (which actually should rather be `velocity-tools-view-servlet`). I will support any solution that results in a way to use `velocity-tools-view` in a modern version of Tomcat (10+). If you are going to split that project, your newly recommended approach would result (e.g.) in the following four artifacts: - velocity-tools-view-servlet (javax) - velocity-tools-view-jsp (jaxax) - velocity-tools-view-servlet-jakarta (jakarta) - velocity-tools-view-jsp-jakarta (jakarta) I would be totally fine with that. This sounds like more than a simple pull request. You probably don't want me trying to change the module names and creating new ones. I'd be willing to try, but I'll stand by for your recommendation. > If a developer cannot make the mental stretch to replace some namespace then he shouldn't deal with it at all. While I agree that it would be unnecessary to duplicate the sample/showcase modules, we were all beginner developers at some point. As a courtesy, perhaps you could simply add a comment above the relevant imports to suggest that the examples will also work with the jakarta version of the modules if "javax." is replaced with "jakarta.". At least that would show that it is intentional and prevent any unnecessary questions. IIRC, Freemarker also included some comments like that somewhere. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2312456351 > > > > > I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be [replicated with Ant](https://stackoverflow.com/questions/8433588/retrieve-value-from-text-file-and-replace-a-string-constant-in-another-file-with). > > > > > > > > > > > > Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 > > > > > > > > > Yes, that should work! It looks like the same approach. > > > @michael-o Would you like me to put a PR together for this, or would you prefer to do it? > > > > > > I would appreciate one. Still occupied with some Maven releases... > > Fantastic! I'll post it in a few days (I don't use Maven for builds, so I need to get a special build/test environment together for this). Here are a few takeaways for you why my approach witha single module will *not* work for Velocity Tools: * It violates Maven principals to have one main artifact per module, two different namespaces are two main artifacts * How to solve different sets of dependencies at any scope? You can't. * How to build classifier artifacts of classier artifacts? JAvadoc/sources? You can't. * Downstream consumers doing analysis of dep trees? You can't. There are more arguments I cannot remember ATM, but we have here only one choice: Yet another module in the reactor which just reuses the sources somehow, rewrites them and builds appropriately. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2311094265 > > > > I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be [replicated with Ant](https://stackoverflow.com/questions/8433588/retrieve-value-from-text-file-and-replace-a-string-constant-in-another-file-with). > > > > > > > > > Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 > > > > > > Yes, that should work! It looks like the same approach. > > @michael-o Would you like me to put a PR together for this, or would you prefer to do it? > > I would appreciate one. Still occupied with some Maven releases... Fantastic! I'll post it in a few days (I don't use Maven for builds, so I need to get a special build/test environment together for this). -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2311085524 > > > I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be [replicated with Ant](https://stackoverflow.com/questions/8433588/retrieve-value-from-text-file-and-replace-a-string-constant-in-another-file-with). > > > > > > Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 > > Yes, that should work! It looks like the same approach. > > @michael-o Would you like me to put a PR together for this, or would you prefer to do it? I would appreciate one. Still occupied with some Maven releases... -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310675389 > > I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be [replicated with Ant](https://stackoverflow.com/questions/8433588/retrieve-value-from-text-file-and-replace-a-string-constant-in-another-file-with). > > Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 Yes, that should work! It looks like the same approach. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310672841 > > > > @ct-parker Thanks for the offer. I am waiting for a Core release first. > > > > > > > > > @michael-o I greatly appreciate your reconsideration of this issue. I am willing to continue the discussion as needed here or in the JIRA issue, so please just let me know of any support you need on this, and I will jump in. I am eager to resolve this. > > > > > > Please point me to the exact commit you want me to look at and propose as well. I want to check the approach taken. > > It was [apache/freemarker#104](https://github.com/apache/freemarker/pull/104), which had a single commit: [apache/freemarker@c88ed78](https://github.com/apache/freemarker/commit/c88ed780669c53ce3979697bf4173cf53d199e6d) > > At some time in the past, the FreeMarker project switched their build process from Ant to Gradle, which gave them the option of using a code generator for solving this issue. > > Essentially, the code generator copies the source code for the package/classes of interest into a package named "freemarker.ext.jakarta" and does a search/replace on javax.servlet to jakarta.servlet prior to compiling. See [this part](https://github.com/apache/freemarker/pull/104/commits/c88ed780669c53ce3979697bf4173cf53d199e6d#diff-c0dfa6bc7a8685217f70a860145fbdf416d449eaff052fa28352c5cec1a98c06) in particular. I'm not suggesting using a code generator, but rather simply duplicating the packages into a jakarta package and search/replace for the "javax." imports. > > The build itself apparently brings both the Servlet 3.1 and 6.0 APIs in as compile dependencies, but after compiling either version may be used in downstream projects. > > The FreeMarker project commentary is [here](https://freemarker.apache.org/docs/versions_2_3_33.html#version_hisotry_freemarker_183_java_side): > > > [FREEMARKER-218](https://issues.apache.org/jira/browse/FREEMARKER-218): Servlet, and JSP support classes now has a Jakarta variant in the new freemarker.ext.jakarta.jsp and freemarker.ext.jakarta.servlet packages. These are the slightly modified copies of freemarker.ext.servlet, freemarker.ext.jsp (which are also present in the same FreeMarker jar), that depend on the Jakarata API-s (jakarta.servlet, jakarta.servlet.jsp, jakarta.el, etc.), instead of the tradition javax Serlvet/JSP/EL API-s. There's also freemarker.ext.jakarta.servlet.WebappTemplateLoader, which is the Jakarta equivalent of freemarker.cache.WebappTemplateLoader. (Note that Jakarta packages/classes are not visible in the FreeMarker source code, as they are generated during build from the pre-Jakarta classes.) > > I'm not familiar with the velocity-tools-view build process, but this is an approach that could easily be [replicated with Ant](https://stackoverflow.com/questions/8433588/retrieve-value-from-text-file-and-replace-a-string-constant-in-another-file-with). Do you consider it similar to mine: https://github.com/michael-o/activedirectory-dns-locator/blob/9dff8dfb3fce8890a8166be6de89b9c05ef77108/pom.xml#L92 -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310635321 > > > @ct-parker Thanks for the offer. I am waiting for a Core release first. > > > > > > @michael-o I greatly appreciate your reconsideration of this issue. I am willing to continue the discussion as needed here or in the JIRA issue, so please just let me know of any support you need on this, and I will jump in. I am eager to resolve this. > > Please point me to the exact commit you want me to look at and propose as well. I want to check the approach taken. It was https://github.com/apache/freemarker/pull/104, which had a single commit: https://github.com/apache/freemarker/pull/104/commits/c88ed780669c53ce3979697bf4173cf53d199e6d They used a code generator (I think FreeMarker itself, with Kotlin and Gradle) which I'm not that familiar with, so I think the end result of the code generation which ends up in "freemarker-src-2.3.33\build\generated\freemarker-jakarta-servlet\main\java\freemarker\ext\jakarta" is the important part. Essentially, the code generator copies the source code for the package/classes of interest into a package named "freemarker.ext.jakarta" and does a search/replace on javax.servlet to jakarta.servlet prior to compiling. See [this part](https://github.com/apache/freemarker/pull/104/commits/c88ed780669c53ce3979697bf4173cf53d199e6d#diff-c0dfa6bc7a8685217f70a860145fbdf416d449eaff052fa28352c5cec1a98c06) in particular. I'm not suggesting using a code generator, but rather simply duplicating the packages into a jakarta package and search/replace for the "javax." imports. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310556115 > > @ct-parker Thanks for the offer. I am waiting for a Core release first. > > @michael-o I greatly appreciate your reconsideration of this issue. I am willing to continue the discussion as needed here or in the JIRA issue, so please just let me know of any support you need on this, and I will jump in. I am eager to resolve this. Please point me to the exact commit you want me to look at and propose as well. I want to check the approach taken. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310321324 > @ct-parker Thanks for the offer. I am waiting for a Core release first. @michael-o I greatly appreciate your reconsideration of this issue. I am willing to continue the discussion as needed here or in the JIRA issue, so please just let me know of any support you need on this, and I will jump in. I am eager to resolve this. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310317627 @ct-parker Thanks for the offer. I am waiting for a Core release first. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ct-parker commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-2310310542 > I can easily explain why I have immediate closed this out: Before opening a PR for such a high-impact change I would expect a discussion in the JIRA issue about a possible solution for everyone. You just dropping off this PR which solves _your_ problem by changing the code in main leaving _everyone else_ behind which do rely on `javax` namespace for the years to come. This is not acceptable. I totally understand that people including you want to make use of `jakarta` namespace, but this is not the right approach. @ppodgorsek This is zero modernization Velocity, it is merely a namespace change, that's it. > > Make a reasonable proposal and I will have a look at this. I wholeheartedly support the sentiment of @ghueller, @ChristopherSchultz, and @ppodgorsek , and I also understand the perspective of @michael-o in the desire to not break the current API. I have proposed a "yes, and" solution in my related comment on [VELTOOLS-202](https://issues.apache.org/jira/projects/VELTOOLS/issues/VELTOOLS-202) that provides a clean path for **supporting Servlet API 3.0 through 6.0 (inclusively) in the same build**. The Apache FreeMarker team recently took this approach in their update from 2.3.32 to 2.3.33, which **does not break any backwards compatibility** and opens the path for developers who need to use a modern Servlet API in their projects. As demonstrated in their minor version update, they did not consider this to be a major change, as it did not affect any dependent projects. They did, however, consider this to be a Major Priority. See [FREEMARKER-218](https://issues.apache.org/jira/browse/FREEMARKER-218). I have successfully migrated Apache Tiles 3 to Servlet API 6.0 (which relies on this project) and have found that it works perfectly in Tomcat 10.1. I feel that this is a very reasonable approach that cuts through all arguments against providing support for Servlet API 6.0 in velocity-tools-view. In light of this proven solution, I respectfully request that you reopen this issue. I would be happy to work with @michael-o on a PR that works toward this solution. -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
michael-o commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-1883045291 > I can't believe that this has been turned down. this is not the problem of one committer but of everyone who wants to keep his software up-to-date. Currently, velocity-tools is preventing this (and as this commit shows, intentionally). I hope you have read my explanation... -- 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
Re: [PR] VELOCITY-942 - VelocityViewServlet extending from jakarta.servlet instead of javax.servlet [velocity-tools]
ghueller commented on PR #15: URL: https://github.com/apache/velocity-tools/pull/15#issuecomment-1883038445 I can't believe that this has been turned down. this is not the problem of one committer but of everyone who wants to keep his software up-to-date. Currently, velocity-tools is preventing this (and as this commit shows, intentionally). -- 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
