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

feat: introduces jakarta servlet protocol module #246

Merged
merged 4 commits into from Jul 6, 2020
Merged

feat: introduces jakarta servlet protocol module #246

merged 4 commits into from Jul 6, 2020

Conversation

starksm64
Copy link
Member

This updates the servlet protocol to support the Jakarta EE 9 jakarta.* package based Servlet 5 APIs

Signed-off-by: Scott M Stark <starksm64@gmail.com>
Signed-off-by: Scott M Stark <starksm64@gmail.com>
Signed-off-by: Scott M Stark <starksm64@gmail.com>
@manovotn
Copy link
Contributor

Fixes #248

This PR and #250 are duplicated efforts. Since this one came first, I closed mine and we can go with this one.
@bartoszmajsak can you review this PR, please?

Meanwhile, I have based my Tomcat adapter work on this PR - arquillian/arquillian-container-tomcat#87

@bartoszmajsak
Copy link
Member

@starksm64 thanks for this new module. Can you give reviewers edit rights to this branch. This would avoid roundtrips for updating this PR against latest master.

@starksm64
Copy link
Member Author

@starksm64 thanks for this new module. Can you give reviewers edit rights to this branch. This would avoid roundtrips for updating this PR against latest master.

I have sent you an invite with write access

@starksm64
Copy link
Member Author

@bartoszmajsak, can you take a look at merging this and getting another arq core release out please?

@bartoszmajsak
Copy link
Member

Sure, I will make that happen tomorrow. Sorry for keeping this one in limbo - got some personal issues to take care of last few days.

Copy link
Member

@bartoszmajsak bartoszmajsak left a comment

Choose a reason for hiding this comment

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

Thank you for this work. Much appreciated. I see that only the following files are jakarta-ee specific:

  • ServletContextRegistrar.java
  • ServletContextResourceProvider.java
  • AbstractServerBase.java
  • ServletTestRunner.java

the rest rely on Arquillian and ShrinkWrap APIs. This makes me wonder if making some common module would be beneficial here. Not necessary as part of this PR and I can take care of it. WDYT @starksm64 @manovotn?

@manovotn
Copy link
Contributor

manovotn commented Jul 3, 2020

@bartoszmajsak I am not against it just not sure it's worth it. Any further development/changes are likely to take place in this new module. Might as well have it all in one place. I know it is duplicating, but any changes to those files would eventually lead us to move them here anyway. Is there so many of them?

Also, note that the tests in this new module (in this PR) are not executed ATM because they require JDK 11. We should IMO setup travis to run that with 11.

@starksm64
Copy link
Member Author

Right, it is just a question of whether it is worth the time. There certainly is a significant amount of redundancy between the two servlet protocols though.

@bartoszmajsak
Copy link
Member

Also, note that the tests in this new module (in this PR) are not executed ATM because they require JDK 11. We should IMO setup travis to run that with 11.

I'm working on it.

@bartoszmajsak bartoszmajsak changed the title Update the servlet protocol to support Jakarta EE 9 / Servlet 5.0 APIs feat: introduces jakarta servlet protocol module Jul 6, 2020
@bartoszmajsak bartoszmajsak merged commit 2f91caf into arquillian:master Jul 6, 2020
@bartoszmajsak
Copy link
Member

Added JDK 11 build in #255

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.

None yet

3 participants