Skip to content

Conversation

@eduarddrenth
Copy link
Contributor

For discussion on the diff, see https://issues.apache.org/jira/browse/VELTOOLS-202

@michael-o michael-o marked this pull request as draft July 18, 2024 19:14
// Really, who cares?
throw new UnsupportedOperationException("This class works only with JSP 2.1");
}

Copy link
Member

Choose a reason for hiding this comment

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

Can you figure out frim with JSP API version this is not required anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since 4.0.0, 3.1.1 still has it. See also https://projects.eclipse.org/projects/ee4j.jsp/releases/4.0.0

Copy link
Member

Choose a reason for hiding this comment

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

Good, thanks. @arkanovicz I wonder whether we should raise to Servlet 4.0 before that.

import java.io.Writer;

import javax.servlet.jsp.JspWriter;
import jakarta.servlet.jsp.JspWriter;
Copy link
Member

Choose a reason for hiding this comment

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

Given the comments at class level, I wonder whether we still need this file...

// Really, who cares?
throw new UnsupportedOperationException("This class works only with JSP 2.1");
}

Copy link
Member

Choose a reason for hiding this comment

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

Good, thanks. @arkanovicz I wonder whether we should raise to Servlet 4.0 before that.

@michael-o
Copy link
Member

@arkanovicz I see at least two classes which have been copied from Tomcat. I wonder whether they are still required these days...

@arkanovicz
Copy link
Contributor

@arkanovicz I see at least two classes which have been copied from Tomcat. I wonder whether they are still required these days...

I don't really maintain the jsp module. I released it unchanged last time, but if nobody can help maintain it, we can deprecate it.

@michael-o
Copy link
Member

@arkanovicz I see at least two classes which have been copied from Tomcat. I wonder whether they are still required these days...

I don't really maintain the jsp module. I released it unchanged last time, but if nobody can help maintain it, we can deprecate it.

That would mean that the JSP tags are dead, aren't they?

@arkanovicz
Copy link
Contributor

@arkanovicz I see at least two classes which have been copied from Tomcat. I wonder whether they are still required these days...

I don't really maintain the jsp module. I released it unchanged last time, but if nobody can help maintain it, we can deprecate it.

That would mean that the JSP tags are dead, aren't they?

I released it as is last time because I've been told that jsp hadn't change, so there was no real reason for the module not to be functional. But I never used jsp, never will, and I do not feel very concerned by its fate. Maybe ask the mailing list?

@michael-o
Copy link
Member

@arkanovicz I see at least two classes which have been copied from Tomcat. I wonder whether they are still required these days...

I don't really maintain the jsp module. I released it unchanged last time, but if nobody can help maintain it, we can deprecate it.

That would mean that the JSP tags are dead, aren't they?

I released it as is last time because I've been told that jsp hadn't change, so there was no real reason for the module not to be functional. But I never used jsp, never will, and I do not feel very concerned by its fate. Maybe ask the mailing list?

Yes, let's do so.

@michael-o
Copy link
Member

@eduarddrenth First of all, thank you for the good cooperation. I already see a few points I can cherry-pick and improve the current code on main, but first I need to take care of velocity master as a prep. Please stay tuned.

@eduarddrenth
Copy link
Contributor Author

Thanks also, I enjoyed to play a small part in making velocity available to application servers for jakarta EE 10. I'll keep an eye open.

@michael-o
Copy link
Member

Why did you close? This is viable code.

@michael-o michael-o reopened this Jul 20, 2024
@eduarddrenth
Copy link
Contributor Author

Why did you close? This is viable code.

I thought you did not need it anymore. Is the process?

  • prepare velocity master
  • merge changes into my fork
  • adapt or resubmit jakarta PR
  • merge PR
  • decide how to release jakarta version and release it

@michael-o
Copy link
Member

Why did you close? This is viable code.

I thought you did not need it anymore. Is the process?

* prepare velocity master

* merge changes into my fork

* adapt or resubmit jakarta PR

* merge PR

* decide how to release jakarta version and release it

You misunderstood. I need to keep it open for visibile and future changes.

@michael-o
Copy link
Member

Factored out a bit #21

@eduarddrenth
Copy link
Contributor Author

At some point in the near future I need to decide how to include velocity deps in my jakarta apps. Either I wait for a jakarta release from the apache team, or I decide to keep my fork up-to-date and release to maven central myself. I prefer the first, but when will this release be?

@michael-o
Copy link
Member

michael-o commented Nov 22, 2024

At some point in the near future I need to decide how to include velocity deps in my jakarta apps. Either I wait for a jakarta release from the apache team, or I decide to keep my fork up-to-date and release to maven central myself. I prefer the first, but when will this release be?

I have expressed my opinion in this regard in the other PR already: #15 (comment)

@arkanovicz
Copy link
Contributor

Most of the standard containers are maintaining old versions for javax and switching to jakarta in their new versions.

I think that the easiest course of action is to do the same, because there is an underneath constraint about the target JVM version, and it will avoid us the hassle of maintaining different modules on the same sources. So my plan is:

  • to release a 3.2 tools version on javax
  • to release a 4.0 tools version on jakarta with a new velocity-master targeting the jvm 11

@michael-o , in this thread, you said:

There is another course of action, now that I think of it: maintain the tools 3.1.x on javax and switch on jakarta for 3.2+.

I am against that. Cutting off people for the years to come for a long waited update is not right. Both Jetty and Tomcat will support javax for quite some time.

But there is no reason why we cannot backport every new feature to the 3.x branch, so users wouldn't be cut off, would they?

And otherwise, I don't really know how to handle the JVM target difference.

@michael-o
Copy link
Member

Most of the standard containers are maintaining old versions for javax and switching to jakarta in their new versions.

I think that the easiest course of action is to do the same, because there is an underneath constraint about the target JVM version, and it will avoid us the hassle of maintaining different modules on the same sources. So my plan is:

  • to release a 3.2 tools version on javax
  • to release a 4.0 tools version on jakarta with a new velocity-master targeting the jvm 11

@michael-o , in this thread, you said:

There is another course of action, now that I think of it: maintain the tools 3.1.x on javax and switch on jakarta for 3.2+.

I am against that. Cutting off people for the years to come for a long waited update is not right. Both Jetty and Tomcat will support javax for quite some time.

But there is no reason why we cannot backport every new feature to the 3.x branch, so users wouldn't be cut off, would they?

And otherwise, I don't really know how to handle the JVM target difference.

The proposal and discussion was to drop javax altogether which I opposed. My proposal was to support both through on the fly code rewrite if possible. Your approach with backports works as well. At the end I wanted a low effort method to serve both.

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.

3 participants