[seam-issues] [JBoss JIRA] Commented: (SEAMINTL-1) TimeZone handling based on Seam 2

[ https://jira.jboss.org/jira/browse/SEAMINTL-1?page=com.atlassian.jira.plu... ] Pete Muir commented on SEAMINTL-1: ---------------------------------- Here's a few comments: * In general formatting is not correct in places , should put braces on a new line etc. Use the formatting template. +public class AvailableTimeZones + @Produces + @TimeZones + private List<TimeZone> timeZones = null; Not convinced we need a qualifier here, otherwise we end up with @Inject @TimeZones List<TimeZone> timezones; which breaks DRY + @Inject Should be @PostConstruct + public void init() { @Produces @DefaultTimeZone @Named + private TimeZone defaultTimeZone; Again, @DefaultTimeZone seems a bit verbose, why not @Default? + + @Inject + public void init() Should be @PostConstruct public class TimeZoneWrapper extends TimeZone Not quite sure what is wrong here, but something is. At least, base this on a Forwarding class as the base (it will extract the wrapping logic from the extra logic you add). This class looks a bit like a holder class, is that correct? Tests need to be placed in a tests sub package (not clear in the guidelines so far, updated) What is the purpose of +public class TimeZoneSelectedEvent? Why do we not just raise an event like @Selected Timezone tz? -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira