+1 to graduate but there are lots of minor things to work on: - No consistent policy for exception handling - for example SQLTimestampConverter catches Throwable and rethrows it as ProvisionException but CharsetConverter do not catch anything at all. Should we catch at all or Guice handles this itself? - No need to catch Errors in such places in any case (catch Exception is enough IMHO); - PropertiesConverter converts String to bytes using ISO-8859-1 and then uses Properties.load() which converts that back to chars. Maybe just use Properties.load(new StringReader(value))? Someone already handled encoding problems and passed String to Guice so why mess with it one more time? - DateConverter is mutable - this is a bad idea itself IMHO but also there is no clean way to set locale and timezone. Maybe there is room for a simple DSL or builder for that module.
Hope that helps =) 2013/2/20 Simone Tripodi <[email protected]> > > +0. Haven't looked at the code. > > Any help will be really appreciated here, Mikhail, I invite you to > participate as much as you can on Onami extensions development :) > > All the best, keep up the good work! > -Simo >
