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

#134 Add Spring-Boot integration #154

Merged
merged 1 commit into from Feb 12, 2016

Conversation

Projects
None yet
3 participants
@marceloverdijk
Member

marceloverdijk commented Feb 9, 2016

Here is the Spring Boot integration code.

We need to give some special attention to spring-boot-starter/template.mf:

..
Import-Template:
 com.github.heneke.thymeleaf.togglz.*;version="0",
 javax.servlet.*;version="0",
 javax.validation.*;version="0",
 org.springframework.*;version="[3.0,4.0)",
 org.springframework.boot.*;version="0",
 org.springframework.security.*;version="[2.0,4.0)",
 org.togglz.*;version="${osgi.bundles.version:default}"

I'm not familiar with this bundlor template.mf format.
I took examples from spring-core and spring-security regarding versions.
For the rest I took "0" as I noticed in other files as well.
Does 4.0 mean that it should be in 4.0.x range? Spring is already in 4.2.x range so I then wonder if template.mf contents are correct?

I will create another PR in the togglz-site repo containing the documentation later.

Marcel Overdijk
<groupId>org.jboss.spec.javax.servlet</groupId>
<artifactId>jboss-servlet-api_3.0_spec</artifactId>
<scope>provided</scope>
</dependency>

This comment has been minimized.

@php-coder

php-coder Feb 9, 2016

Do we really need this dependency? Is it will be added to any application that uses this module? Hm... Looks like overkill and dependency to something from org.jboss maybe isn't good IMHO.

Do we need it for javax.servlet.* classes? What dependency Spring Boot uses for that?

This comment has been minimized.

@marceloverdijk

marceloverdijk Feb 10, 2016

Member

It is provided scope so I don't see the issue. This is how Togglz project depends on servlet spec.
It just assumes javax.servlet.* is available. Only needed to compile project, and actual servlet implementation provided by Boot (or it's appropriate starter).

This comment has been minimized.

@chkal

chkal Feb 12, 2016

Member

I think <optional> dependencies are fine. But I'm curious. Why is there an optional dependency on Hibernate Validator?

Togglz uses the JBoss provided Java EE spec Maven artifacts. Actually it doesn't matter so much which one is used as it is provided scoped.

<hamcrest.version>1.3</hamcrest.version>
<hibernate-validator.version>5.2.2.Final</hibernate-validator.version>
<spring-boot.version>1.3.2.RELEASE</spring-boot.version>
<spring-security.version>4.0.3.RELEASE</spring-security.version>

This comment has been minimized.

@php-coder

php-coder Feb 9, 2016

Spring Boot parent module already has versions for ham crest, hibernate validator and spring security. Could we use dependency from parent module? If not then we will always update them with each new Spring Boot release. Also it's possible to mix different versions of the same dependency... Hm.. Please, correct me if I'm wrong.

This comment has been minimized.

@marceloverdijk

marceloverdijk Feb 10, 2016

Member

The parent module is not boot, but Togglz. Also note dependencies are optional so I don't see issue here.
In theory the actual dependnecy will be provided by Boot itself. I just need them to compile the code.

This comment has been minimized.

@chkal

chkal Feb 12, 2016

Member

I agree that the actually version isn't so important because of the optional nature of the dependencies. Basically it is more some kind of "minimal" version that Togglz supports. That's why the Togglz Spring module for example depends on a very old Spring version.

@chkal

This comment has been minimized.

Member

chkal commented Feb 12, 2016

@marceloverdijk Thank you very much for the pull request. I think it is fine.

I'll merge it now. Perhaps we get some more people to test as it is now part of the official snapshots.

Thanks a lot for your help. 🍻

chkal added a commit that referenced this pull request Feb 12, 2016

@chkal chkal merged commit f0af3a1 into togglz:master Feb 12, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment