Hi John, Merry Christmas to you as well! > However, I only see support for implicit array converters
Yes, that part was clarified by Emily on the ConfigJSR hangout. She intentionally intended the functionality to be a Converter, not baked in in ConfigImpl. That way someone could also provide a custom Converter for Dog[], which would then take precedence. I'm not sure myself whether this is such an important use case. For normal situations it makes no difference, but if one registers a custom coverter for an array type, then it is. Btw, your array handling impl seems to be much smaller than mine. Guess we could improve my version a bit - happy if you could take a look at it ;) > but I think you should look at the approach I took on the escape character > handling in MP version. > I do end up looping around the value twice (regex + foreach) I think the regexp version is much shorter in code and thus probably easier to understand. Otoh the manual char loop is much faster at execution time. It doesn't need any regexp (performance intense) and only requires to loop over the string once (due to a lookahead logic). Both should deliver the same results though. > One big difference I see, in my impl when you want a array, set or list, I go > string -> list first, then convert to either array or set if need be. In the > JSR case, I see it is doing string -> list -> array then to list/set if need > be Yes, I tried to avoid auto-boxing if possible. Not sure I 100% succeeded though and whether it really makes sense. A review of this part would certainly be a good idea. I just wanted to make the TCK pass. > Perhaps we can blend the two together? +1 Would be much easier to maintain the two versions if we manage to keep them in sync! txs and LieGrue, strub > Am 25.12.2017 um 15:51 schrieb John D. Ament <[email protected]>: > > Hi & Merry Christmas guys! > > I believe you applied implicit converters to both branches. However, I only > see support for implicit array converters (maybe that's how you aimed to do > array types?). The actual implementation is pretty much the same, I would > just move the array logic into its own converter for consistency; but I think > you should look at the approach I took on the escape character handling in MP > version. I do end up looping around the value twice (regex + foreach) > > One big difference I see, in my impl when you want a array, set or list, I go > string -> list first, then convert to either array or set if need be. In the > JSR case, I see it is doing string -> list -> array then to list/set if need > be. > > Perhaps we can blend the two together? > > John > > On Mon, Dec 25, 2017 at 5:24 AM Mark Struberg <[email protected]> wrote: > Hi! > > +1 in general > > @John, please also take a look at the ConfigJSR branch. > It already contains many of the functionality already as Emily just copied > over my changes from JSR-382 to mp-config. > > E.g all the implicit converters and array stuff is already done. > Happy to have a hangout and do a pair review of it and then unify it again. > > LieGrue, > strub > > > Am 24.12.2017 um 08:23 schrieb Romain Manni-Bucau <[email protected]>: > > > > Sounds good. Thanks for the heads up > > > > Le 24 déc. 2017 05:02, "John D. Ament" <[email protected]> a écrit : > > Hey all > > > > I just pushed up the last of the changes in the Geronimo config trunk that > > would align to the MP Config 1.2 spec. There's still a bit of clean up I > > want to aim for, so maybe after the new year we can plan to cut a release > > of Config 1.1? > > > > You'll note in my commit, I moved most of the logic around converter > > registration from ConfigImpl into DefaultConfigBuilder. I think this works > > out a bit better, this way we only have to register converters that are > > prioritized. I want to align Implicit converters into the > > MicroProfileTypedConverter class, to clean up the code a bit more. > > > > John >
