Skip to content

Conversation

@stephanrauh
Copy link

This pull request adds JPA and JUnit and makes the index.xhtml nicer and hopefully more useful. I also added a Json serializer because I noticed that I couldn't pass POJOs to a REST service.

This PR has been tested with Tomcat 8.5.21 and Tomcat 9.0.0 M27.

…and made the index.xhtml nicer and hopefully more useful
Copy link
Owner

@jdesmet jdesmet left a comment

Choose a reason for hiding this comment

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

We should not hardcode the application context path as in "http://127.0.0.1:8080/zakee-web/...". For a better solution, please see https://stackoverflow.com/a/6878393/744133

jdesmet and others added 2 commits September 28, 2017 00:44
@stephanrauh
Copy link
Author

Of course, you're right. Done.

Along the way, I also corrected the formatting of the index.xhtml. It was sort of scrambled by my IDE.

Copy link
Owner

@jdesmet jdesmet left a comment

Choose a reason for hiding this comment

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

I think that is about the only think left to merge in your changes from your pull request.

<artifactId>jersey-container-servlet</artifactId>
<version>2.25</version>
</dependency>
<dependency>
Copy link
Owner

Choose a reason for hiding this comment

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

There should be no explicit dependencies on repackaged libraries. They are intended internal only to other dependencies that applied it in a transitive manner.

@stephanrauh
Copy link
Author

@jdesmet Your change request is fine by me. The only problem is that I'm currently abroad. So either we have to wait three weeks, or you remove the offending dependency yourself. You seem to know what to do.

Let me explain why I added the dependency: the program either didn't compile or run without the repackaged library. I forgot what happened precisely, but it was one of the two options. So I decided to add the dependency matching the requirements as good as possible. I don't have the slightest idea why the transitive dependency isn't added automatically. Maybe I've missed a third necessary library?

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