Hi Mikhail! thanks a lot for having a look at the [converters] codebase! But lets' clarify that graduate from /sandbox to /trunk means that the codebase respects the initial quality standards required by Onami, such as applied license, right package, onami build integration and so on; it doesn't mean that it is ready to be released :)
Given that, please raise an issue to each problem you noticed - that will makes absolutely easier to keep track of TODOs - and feel free to assign to yourself and provide a fix! :) Many thanks in advance, all the best, -Simo http://people.apache.org/~simonetripodi/ http://simonetripodi.livejournal.com/ http://twitter.com/simonetripodi http://www.99soft.org/ On Thu, Feb 21, 2013 at 1:25 PM, Mikhail Mazursky <[email protected]> wrote: > +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 >>
