On Wed, Jun 25, 2008 at 4:30 PM, Mike Kienenberger <[EMAIL PROTECTED]> wrote:
> On 6/25/08, simon <[EMAIL PROTECTED]> wrote: > > > > On Wed, 2008-06-25 at 10:59 -0400, Mike Kienenberger wrote: > > > On 6/25/08, [EMAIL PROTECTED] <[EMAIL PROTECTED]> > wrote: > > > > > s:validateCompareTo > > > > > > > > > And all that code about independently converting a component's > submitted > > > > value makes me a little nervous. It looks ok, but has anyone > properly > > > > reviewed it? > > > > Previouly to move s:validateCompareTo, I solved some issues. In this task, I made some very simple examples on myfaces-commons-examples and works well. > > > > The code was pretty much copied verbatim from the Myfaces JSF core. > > > In any case, the component has been used by several people for several > > > years now, and no one has complained. If it's a problem down the > > > road, I'm sure someone will open a JIRA issue and we can address it at > > > that time. > > > > > > It occurred to me after looking at the code that an alternative might be > > to queue an event from the validator. The event should be processed only > > at the end of the validation phase, at which time all conversion has > > already been done. That could simplify the converter - and it does feel > > more elegant to me than having one validator poking around inside > > another one. > > Yes, that idea sounds better. I admit that it's a bit scary that the > component has to rely on the implementation code from MyFaces. Even > though it uses public APIs, in practice, it might break on another > implementation. > > > > > > > In tomahawk core, the related files should be moved from > sandbox/core to > > > > core. In tomahawk core12, a new dependency to > myfaces-commons-converters > > > > 1.2.x and myfaces-commons-validators 1.2.x should be added, so the > tomahawk > > > > core12 tld reference validators and converters from these projects. > This > > > > introduce a change because > > > > > the validatorId and converterId for this components changes > (because this > > > > converters are defined on myfaces-commons) only on core12. > > > > > > > > I don't like the idea of tomahawk1.x having these components > internally, > > > > while tomahawk2.x uses the version from commons. It's ugly and > confusing. > > > > > > > > While code duplication is never nice, I think it would be better > for > > > > tomahawk 1.1.x and 1.2.x to continue to have these components > internally, > > > > and for commons to have a separate version. It also means that > commons can > > > > clean up the API without breaking tomahawk users. Yes, it does mean > having > > > > to apply fixes in two places (tomahawk, commons) but so does the > alternative > > > > (tomahawk 1.x, commons). > > > > > > I say we mark the tomahawk validator and converter components as > > > deprecated, leaving them as they are and where they are, and copy > > > everything to commons. > > > > > > No one is forced to upgrade unless they need something fixed or > > > enhanced, but we don't maintain it in multiple places. > > > > > > I've lost track now. Is there going to be a commons 1.1.x version or > > not? > > > > If there is a commons-1.1.x then I agree my suggestion was completely > > wrong. These are just sandbox components. So we could move them directly > > into commons, and not promote them into tomahawk at all. > > > > Or possibly leave the original in the sandbox for a while, marked as > > deprecated so people have some time to migrate. But I'm not sure that is > > needed for sandbox features. > > > > And there would be no dependency from tomahawk to commons. If people > > want these, they add the commons lib to their path. The commons libs do > > contain their own tld (with its own namespace), yes? > > Does it really matter if there's a commons 1.1? Tomahawk 1.1 is fast > approaching the end of its lifecycle. > I agree. The idea is going forward with tomahawk 1.2 (or myfaces-extensions-xxxx), so why waste time on commons 1.1 (taking into account what the community think about it) > > Leave it deprecated in Tomahawk 1.1 (and sandbox). We would want to > do that anyway for backwards compatibility. Drop it from Tomahawk > post-1.1 (and sandbox). >
