Polonious / Regimo

4 stars 6 forks source link

new user confirmation email enhancement #15

Closed willzou closed 12 years ago

RalfWeinand commented 12 years ago

Hi Will,

I added a few comments.

Best regards, Ralf

On Tue, May 8, 2012 at 4:36 PM, willzou < reply@reply.github.com

wrote:

You can merge this Pull Request by running:

git pull https://github.com/willzou/Regimo master

Or you can view, comment on it, or merge it online at:

https://github.com/Polonious/Regimo/pull/15

-- Commit Summary --

  • test
  • Merge branch 'master' of git@github.com:willzou/Regimo.git

-- File Changes --

M regimo-web/src/main/java/au/com/regimo/web/signup/SignupController.java (7)

-- Patch Links --

https://github.com/Polonious/Regimo/pull/15.patch https://github.com/Polonious/Regimo/pull/15.diff


Reply to this email directly or view it on GitHub: https://github.com/Polonious/Regimo/pull/15

tedliang commented 12 years ago

We have profile feature. Please use it for any hard codes. Also, most of the business logic should be in the services instead of controller. The controllers should only contain logic for UI.

willzou commented 12 years ago

There is no signup service, the only relevant service is userservice which is in core model. I plan to build up some service in server model which will deal with the regimo service

RalfWeinand commented 12 years ago

Hi Will, If there is no service create one, don't wait for someone to point this out during a review. We are following the MVC model and want clean code and a clean structure. Always. Thanks.

tedliang commented 12 years ago

You have implemented the i18n feature for Austjobs before. Regimo is using a JSP and Bean Validation. Here is a good tutorial: http://blog.inflinx.com/2010/03/10/jsr-303-bean-validation-using-spring-3/