Sold! Working on it. On Fri, May 15, 2015 at 2:42 PM, andrea del bene <[email protected]> wrote:
> +1 also for me > > > On 15/05/2015 14:15, Martin Grigorov wrote: > >> +1 from me too >> >> Martin Grigorov >> Wicket Training and Consulting >> https://twitter.com/mtgrigorov >> >> On Fri, May 15, 2015 at 3:10 PM, Martijn Dashorst < >> [email protected]> wrote: >> >> I'm +1 for Guillaume's proposal, even if it breaks API late in the >>> game. I'd rather have that than having to go through all >>> StringResourceModel's constructors trying to figure out if they break >>> silently. >>> >>> Martijn >>> >>> >>> On Thu, May 14, 2015 at 8:25 PM, Guillaume Smet >>> <[email protected]> wrote: >>> >>>> Hi, >>>> >>>> Sven's proposal was to create a new PropertyResourceModel which is IMHO >>>> >>> orthogonal to whether we decide to fix the issue we have with the new >>> StringResourceModel signature. >>> >>>> Sven's proposal was to remove the MessageFormat support altogether and I >>>> >>> think it's a quite disruptive change. >>> >>>> I think moving StringResourceModel to a more fluid interface would be >>>> >>> useful anyway to avoid having applications silently failing after the >>> migration to 7. >>> >>>> I can work on a pull request if we agree on this. >>>> >>>> -- >>>> Guillaume >>>> >>>> Le 14 mai 2015 à 08:55, Martin Grigorov <[email protected]> a écrit >>>>> >>>> : >>> >>>> Hi, >>>>> >>>>> I am not sure why Sven didn't push his proposal (StringResourceModel): >>>>> http://markmail.org/message/35vkmmao5dxrmno2 >>>>> In this mail thread there was a discussion about such change (fluent >>>>> >>>> API). >>> >>>> Martin Grigorov >>>>> Wicket Training and Consulting >>>>> https://twitter.com/mtgrigorov >>>>> >>>>> On Thu, May 14, 2015 at 12:42 AM, Martijn Dashorst < >>>>> [email protected]> wrote: >>>>> >>>>> Sounds like a good idea. We have a special I18N class in our app >>>>>> specifically as a wrapper around StringResourceModel. >>>>>> >>>>>> Martijn >>>>>> >>>>>> On Wed, May 13, 2015 at 11:29 PM, Guillaume Smet >>>>>> <[email protected]> wrote: >>>>>> >>>>>>> I gave it some more thoughts and I admit it's a bit radical but I >>>>>>> >>>>>> think >>> >>>> we >>>>>> >>>>>>> should simply remove all these constructors and give >>>>>>> >>>>>> StringResourceModel >>> >>>> a >>>>>> >>>>>>> fluid interface. >>>>>>> >>>>>>> We could keep 2 constructors: >>>>>>> StringResourceModel(final String resourceKey, final Component >>>>>>> >>>>>> component) >>> >>>> StringResourceModel(final String resourceKey) >>>>>>> >>>>>>> and add the following methods: >>>>>>> setDefaultValue >>>>>>> setModel >>>>>>> setParameters >>>>>>> all these methods returning 'this'. >>>>>>> >>>>>>> We often call the constructors with null values for some of these >>>>>>> parameters anyway and it would be far more readable and less error >>>>>>> >>>>>> prone. >>> >>>> When I upgrade, I really prefer having compilation errors rather than >>>>>>> >>>>>> code >>>>>> >>>>>>> silently failing (currently it might not take the default value and >>>>>>> it >>>>>>> might get an off by one error for the parameters). >>>>>>> >>>>>>> Thoughts? >>>>>>> >>>>>>> On Wed, May 13, 2015 at 1:44 PM, Guillaume Smet < >>>>>>> >>>>>> [email protected]> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>> Yup, that's exactly the issue I had. >>>>>>>> >>>>>>>> It's not hard to fix in user code. The hard part is to find the >>>>>>>> occurrences of the issue in the code. >>>>>>>> >>>>>>>> >>>>>>>> On Wed, May 13, 2015 at 8:05 AM, Martin Grigorov < >>>>>>>> >>>>>>> [email protected]> >>> >>>> wrote: >>>>>>>> >>>>>>>> https://issues.apache.org/jira/browse/WICKET-5906 sounds related. >>>>>>>>> >>>>>>>>> Martin Grigorov >>>>>>>>> Wicket Training and Consulting >>>>>>>>> https://twitter.com/mtgrigorov >>>>>>>>> >>>>>>>>> On Mon, May 11, 2015 at 11:26 PM, Guillaume Smet < >>>>>>>>> [email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> Still working on the migration to 7. >>>>>>>>>> >>>>>>>>>> We used this pattern with Wicket 6: >>>>>>>>>> new StringResourceModel(propertyModel.getObject(), null, >>>>>>>>>> propertyModel.getObject(), objectModel, secondaryObjectModel)) >>>>>>>>>> which was directed to the following constructor: >>>>>>>>>> StringResourceModel(key, model, defaultValue, Object... >>>>>>>>>> parameters) >>>>>>>>>> >>>>>>>>>> When migrating to 7, this pattern is silently directed to the >>>>>>>>>> >>>>>>>>> following >>>>>> >>>>>>> constructor: >>>>>>>>>> StringResourceModel(key, model, Object... parameters) >>>>>>>>>> thus the replacements are wrong. >>>>>>>>>> >>>>>>>>>> The fact that our application is silently broken is kinda >>>>>>>>>> annoying. >>>>>>>>>> >>>>>>>>>> I ended up moving to: >>>>>>>>>> new StringResourceModel(propertyModel.getObject(), (IModel<?>) >>>>>>>>>> >>>>>>>>> null, >>> >>>> propertyModel, new Object[] { objectModel, secondaryObjectModel })) >>>>>>>>>> >>>>>>>>>> I don't know if there is something to do about it but I thought I >>>>>>>>>> >>>>>>>>> might >>>>>> >>>>>>> as >>>>>>>>> >>>>>>>>>> well get it out here. >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Guillaume >>>>>>>>>> >>>>>>>>> >>>>>> >>>>>> -- >>>>>> Become a Wicket expert, learn from the best: >>>>>> http://wicketinaction.com >>>>>> >>>>>> >>> >>> -- >>> Become a Wicket expert, learn from the best: http://wicketinaction.com >>> >>> >
