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

Supports the Spring's MessageResolver #63

Merged
merged 2 commits into from Oct 16, 2017

Conversation

Projects
None yet
3 participants
@kazuki43zoo
Copy link
Contributor

kazuki43zoo commented Sep 16, 2017

I want to support the Spring's MessageResolver integrating the MessageSource.
What do you think?

@serac

This comment has been minimized.

Copy link
Member

serac commented Oct 6, 2017

Looks good to me. Thanks for contributing this -- I can imagine it will be very useful when Passay is used in a Spring Framework app.

pom.xml Outdated
<artifactId>spring-context-support</artifactId>
<version>4.3.11.RELEASE</version>
<optional>true</optional>
<scope>provided</scope>

This comment has been minimized.

@dfish3r

dfish3r Oct 6, 2017

Member

What does optional do when used with provided?
I've only used optional with compile scope artifacts.

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Oct 6, 2017

Contributor

Thanks for your comment.

I think the Spring's artifact not required for using the core feature of Passy, therefore I've set true for the optional flag. And I've set the provided at the scope to indicates that user can select Spring version.
What do you think?

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Oct 6, 2017

Contributor

But It might be better only set optional=true.
Maven document says about provided as follows:

This is much like compile, but indicates you expect the JDK or a container to provide the dependency at runtime. For example, when building a web application for the Java Enterprise Edition, you would set the dependency on the Servlet API and related Java EE APIs to scope provided because the web container provides those classes. This scope is only available on the compilation and test classpath, and is not transitive.

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Oct 9, 2017

Contributor

@dfish3r, Should I remove the scope element?

This comment has been minimized.

@dfish3r

dfish3r Oct 16, 2017

Member

No, let's stick with provided for now. I see other projects marking dependencies this way, so we'll go with it.

pom.xml Outdated
@@ -93,18 +93,19 @@
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context-support</artifactId>

This comment has been minimized.

@dfish3r

dfish3r Oct 8, 2017

Member

Why spring-context-support instead of just spring-context?

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Oct 9, 2017

Contributor

Oh... sorry, I misunderstood that the MessageSourceAccessor is in spring-context-support.

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Oct 9, 2017

Contributor

Fix via 74ad91d.

@dfish3r dfish3r merged commit fe1cc92 into vt-middleware:master Oct 16, 2017

@kazuki43zoo kazuki43zoo deleted the kazuki43zoo:support-spring-message-resolver branch Oct 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment