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

Add support to automatic register handler annotated with RegisterActionH... #292

Merged
merged 4 commits into from Jul 18, 2013

Conversation

@igieon
Copy link
Contributor

commented Jul 17, 2013

No description provided.

* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

Not valid license

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

I meant this it not the license headers used in others files

/**
* Annotation bean post processing to register ActionHadlers annotate
* with {@link RegisterActionHandler}.
* @author David Ignjic

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

remove @author, git maintain this information

* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

license header

* with action {@link ActionHandler#getActionType()} and
* validator {@link RegisterActionHandler#validator()}.
*
* @author David Ignjic

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

remove

* @author David Ignjic
*
*/
package com.gwtplatform.dispatch.server.spring.annotation;

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

missing nl

@@ -0,0 +1,7 @@
/**

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

I don't think we use package-info in the library, so I I think it's better to remove it in order to stay consistent

* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

hearder

import com.gwtplatform.dispatch.shared.action.TestResult;

/**
* @author Peter Simun

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

remove author

* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

header

import com.gwtplatform.dispatch.shared.UnsecuredActionImpl;

/**
* @author Peter Simun

This comment has been minimized.

Copy link
@jDramaix

jDramaix Jul 17, 2013

Contributor

remove author

@jDramaix

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Thanks for contributing !

I think we could do the same for the non-spring part of gwt-dispatch ?

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

Need to something to change in my contribution ?
"I think we could do the same for the non-spring part of gwt-dispatch ?" what about this ?

@jDramaix

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Need to something to change in my contribution ?

Put the right license header on all files and remove the @author definition in your javadocs. Check also the length of your lines, they should not exceed 120 chars.

"I think we could do the same for the non-spring part of gwt-dispatch ?" what about this ?

It was just a suggestion/ open question. IIRC, when you define a ActionHandler and that you don't use Spring, you have to bind your handler using :

bindHandler( Action.class, Handler.class, Validator.class ); 

We could maybe implements the same mechanism with guice : simply detect the ActionHandler annotated with RegisterActionHandler and bind them automatically (I guess it should be doable). I just want to offer the same feature whatever your are using spring or not.

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

Ok I modify it and resubmit patch.

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

About guice i need to look into guice more to understand how do it on it.

@jDramaix

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Ok I modify it and resubmit patch.

Just commit your changes in your branch and push your branch in the remote repo, that should update this PR

@branflake2267

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Nice job on the source.

Teamcity pull build is complaining a little about checkstyle yet. Might be the first new line after the first class curly.

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

Could you provide update for checkstyle because now maven checkstyle didn't complain about class curly. Or eclipse formater and clean up for you style. Thanks.

@branflake2267

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Here are the rules for the checkstyle, and one of these can be added to the eclipse plugin https://github.com/ArcBees/GWTP/tree/master/gwtp-build-tools/src/main/resources

*
*/
public class AnnotatedActionBeandHandlerRegistrator implements BeanPostProcessor, Ordered {

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

remove new lines after the first class curly. :) This may be one of the checkstyle complaints.

*/
@Retention(RetentionPolicy.RUNTIME)
public @interface RegisterActionHandler {

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

rm nl

* Annotation to put on handler to automatic register handler
* with action {@link ActionHandler#getActionType()} and
* validator {@link RegisterActionHandler#validator()}.
*

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

rm empty lines in javadocs

*/
@RegisterActionHandler
public class TestAnnotatedActionHandler extends AbstractActionHandler<TestAnnotatedAction, TestResult> {

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

rm nl

/**
*/
public class TestAnnotatedAction extends UnsecuredActionImpl<TestResult> {

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

rm nl

/**
* Annotation bean post processing to register ActionHadlers annotate
* with {@link RegisterActionHandler}.
*

This comment has been minimized.

Copy link
@branflake2267

branflake2267 Jul 17, 2013

Contributor

rm empty line

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

I use https://github.com/ArcBees/GWTP/tree/master/gwtp-build-tools/src/main/resources this one but the eclipse nor the maven checkstyle doesn't complain about this lines.

@branflake2267

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Good question, not sure which items its complaining about, I thought maybe that might be it. :) mvn checkstyle:checkstyle might tell a better story.

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2013

ok here is my command line about checkstyle after remove new lines no checkstyle complain and before alse

C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring [master +1 ~1 -0 !]> git log -2
commit 8d7b2456849fa10d84d7e57a90c04902edfbdc49
Author: David Ignjić <ignjic@gmail.com>
Date:   Wed Jul 17 16:32:22 2013 +0200

    Remove empty new lines

commit 6d7f8fe8a961018183ef5dfa5862dc1f8098da89
Author: David Ignjić <ignjic@gmail.com>
Date:   Wed Jul 17 15:05:48 2013 +0200

    Modify source to  comply with checkstyle
C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring [master +1 ~1 -0 !]> mvn checkstyle:che
ckstyle
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building GWTP Dispatch Server, Spring implementation 1.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-checkstyle-plugin:2.7:checkstyle (default-cli) @ gwtp-dispatch-server-spring ---
[INFO] Starting audit...
Audit done.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.027s
[INFO] Finished at: Wed Jul 17 16:36:18 CEST 2013
[INFO] Final Memory: 17M/217M
[INFO] ------------------------------------------------------------------------
C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring [master +1 ~1 -0 !]> git checkout 6d7f8
fe8a961018183ef5dfa5862dc1f8098da89
M       gwtp-core/gwtp-dispatch-server-guice/src/main/java/com/gwtplatform/dispatch/server/guice/actionhandlervalidator/
LazyActionHandlerValidatorRegistryImpl.java
Note: checking out '6d7f8fe8a961018183ef5dfa5862dc1f8098da89'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at 6d7f8fe... Modify source to  comply with checkstyle
C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring [(6d7f8fe...) +1 ~1 -0 !]> mvn clean ch
eckstyle:checkstyle
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building GWTP Dispatch Server, Spring implementation 1.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.4.1:clean (default-clean) @ gwtp-dispatch-server-spring ---
[INFO] Deleting C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring\target
[INFO]
[INFO] --- maven-checkstyle-plugin:2.7:checkstyle (default-cli) @ gwtp-dispatch-server-spring ---
[INFO] Starting audit...
Audit done.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.354s
[INFO] Finished at: Wed Jul 17 16:36:50 CEST 2013
[INFO] Final Memory: 18M/217M
[INFO] ------------------------------------------------------------------------
C:\Users\davidignjic\Documents\GitHub\GWTP\gwtp-core\gwtp-dispatch-server-spring [(6d7f8fe...) +1 ~1 -0 !]>
@branflake2267

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2013

Hmm... @meriouma do you know what might be going on with checkstyle?

@meriouma

This comment has been minimized.

Copy link
Member

commented Jul 17, 2013

I'm looking at it

@meriouma

This comment has been minimized.

Copy link
Member

commented Jul 17, 2013

Your ArcBees/GWTP git remote seems out of date (hence the conflict). You need to merge ArcBees/GWTP:master in your igieon/GWTP:master branch

Merge remote-tracking branch 'upstream/master'
Conflicts:
	gwtp-core/gwtp-dispatch-server-spring/src/main/java/com/gwtplatform/dispatch/server/spring/actionhandlervalidator/LazyActionHandlerValidatorRegistryImpl.java
@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2013

Merged with master

@christiangoudreau

This comment has been minimized.

Copy link
Member

commented Jul 18, 2013

@imrabti could you do a review?

@imrabti

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

LGTM,
To use this new annotation, we need to declare AnnotatedActionBeandHandlerRegistrator as a Bean Right ?

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2013

Yes, as you can see in test.

christiangoudreau added a commit that referenced this pull request Jul 18, 2013
Merge pull request #292 from igieon/master
Add support to automatic register handler annotated with RegisterActionH...

@christiangoudreau christiangoudreau merged commit f53d899 into ArcBees:master Jul 18, 2013

1 check passed

default TeamCity Build GWT-Platform (GWTP) :: Monitor Pulls finished: Tests passed: 87
Details
@christiangoudreau

This comment has been minimized.

Copy link
Member

commented Aug 13, 2013

This seems undocumented. @imrabti could you please document this feature in the wiki?

@igieon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2013

Provide some example in wiki or what ?

@christiangoudreau

This comment has been minimized.

Copy link
Member

commented Aug 13, 2013

Exactly

christiangoudreau added a commit that referenced this pull request Apr 4, 2014
Merge pull request #292 from igieon/master
Add support to automatic register handler annotated with RegisterActionH...
hpehl pushed a commit to hpehl/GWTP that referenced this pull request Dec 9, 2014
Merge pull request ArcBees#292 from igieon/master
Add support to automatic register handler annotated with RegisterActionH...


Former-commit-id: fec0ac4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.