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.

Sign up for GitHub

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

Jump to bottom

LinkToHandler localized Closes #28 #32

Merged
merged 5 commits into from
Oct 14, 2014
Merged

LinkToHandler localized Closes #28 #32

merged 5 commits into from
Oct 14, 2014

Conversation

dtelaroli
Copy link
Contributor

This PR depends on the caelum/vraptor4#702

Is right to downgrade from @ApplicationScoped to @RequestScoped, since that it depends on the JstlLocalization?

@garcia-jj
Copy link
Member

You can't upgrade to versions not released yet. We need to wait a new release from core before merging this.

@dtelaroli
Copy link
Contributor Author

@garcia-jj I know, but I did that to implement the feature.
So, need I to remove the commit 930cb47?

@garcia-jj
Copy link
Member

Ask @Turini to release a rc2 or wait a final release of core.
On Aug 1, 2014 1:43 PM, "Denilson Telaroli" notifications@github.com
wrote:

@garcia-jj https://github.com/garcia-jj I know, but I did that to
implement the feature.
So, need I to remove the commit 930cb47
930cb47
?

—
Reply to this email directly or view it on GitHub
#32 (comment).

@Turini
Copy link
Member

Turini commented Aug 1, 2014

@dtelaroli, I guess the release will happen starting next week (if everyone
agree with), so I can notice you to update this version and merge here. ok?

@dtelaroli
Copy link
Contributor Author

👌

lucascs
lucascs reviewed Aug 2, 2014
View reviewed changes
pom.xml
@@ -29,7 +29,7 @@
<dependency>
<groupId>br.com.caelum</groupId>
<artifactId>vraptor</artifactId>
<version>4.0.0.Final</version>
<version>4.1.0-RC2-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

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

this PR should not be merged with a SNAPSHOT version here. But this version will be released soon =)

@dtelaroli
Copy link
Contributor Author

@Turini Think you that has time to send other PR on the core before of release, to remove duplicated code as @lucascs said?

@Turini
Copy link
Member

Turini commented Aug 4, 2014

yeap, can you do that until tomorrow?

@dtelaroli
Copy link
Contributor Author

👌
I will do it tonight

@Turini
Copy link
Member

Turini commented Aug 4, 2014

perfect, thks

@dtelaroli dtelaroli mentioned this pull request Aug 4, 2014
Merged
@dtelaroli
Copy link
Contributor Author

@Turini I did rebase this PR after a time absent.
I kept the RC version of other commit.
Still missing something?

@garcia-jj
Copy link
Member

@dtelaroli I think we can update to 4.1.1 (our latest core version).

@dtelaroli
Copy link
Contributor Author

Done @garcia-jj

@Turini
Copy link
Member

Turini commented Oct 14, 2014

Sounds great @dtelaroli. Merging here

Turini added a commit that referenced this pull request Oct 14, 2014
@Turini
Merge pull request #32 from dtelaroli/linkTo
6b32e26 
LinkToHandler localized Closes #28
@Turini Turini merged commit 6b32e26 into caelum:master Oct 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dtelaroli @garcia-jj @Turini @lucascs