You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Guillaume Smet <gu...@gmail.com> on 2015/05/11 22:26:15 UTC
Wicket 7 and StringResourceModel
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
Re: Wicket 7 and StringResourceModel
Posted by Guillaume Smet <gu...@gmail.com>.
https://github.com/apache/wicket/pull/118
I removed a few null, (Component)null... here and there so it's definitely
cleaner than before.
Comments welcome on the API. Will do the legwork to get this committed.
Re: Wicket 7 and StringResourceModel
Posted by Guillaume Smet <gu...@gmail.com>.
Sold! Working on it.
On Fri, May 15, 2015 at 2:42 PM, andrea del bene <an...@gmail.com>
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 <
>> martijn.dashorst@gmail.com> 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
>>> <gu...@gmail.com> 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 <mg...@apache.org> 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 <
>>>>> martijn.dashorst@gmail.com> 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
>>>>>> <gu...@gmail.com> 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 <
>>>>>>>
>>>>>> guillaume.smet@gmail.com>
>>>>>>
>>>>>>> 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 <
>>>>>>>>
>>>>>>> mgrigorov@apache.org>
>>>
>>>> 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 <
>>>>>>>>> guillaume.smet@gmail.com>
>>>>>>>>> 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
>>>
>>>
>
Re: Wicket 7 and StringResourceModel
Posted by andrea del bene <an...@gmail.com>.
+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 <
> martijn.dashorst@gmail.com> 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
>> <gu...@gmail.com> 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 <mg...@apache.org> 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 <
>>>> martijn.dashorst@gmail.com> 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
>>>>> <gu...@gmail.com> 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 <
>>>>> guillaume.smet@gmail.com>
>>>>>> 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 <
>> mgrigorov@apache.org>
>>>>>>> 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 <
>>>>>>>> guillaume.smet@gmail.com>
>>>>>>>> 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
>>
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
+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 <
martijn.dashorst@gmail.com> 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
> <gu...@gmail.com> 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 <mg...@apache.org> 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 <
> >> martijn.dashorst@gmail.com> 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
> >>> <gu...@gmail.com> 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 <
> >>> guillaume.smet@gmail.com>
> >>>> 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 <
> mgrigorov@apache.org>
> >>>>> 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 <
> >>>>>> guillaume.smet@gmail.com>
> >>>>>> 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
>
Re: Wicket 7 and StringResourceModel
Posted by andrea del bene <an...@gmail.com>.
Thank you! I also think it's a good solution.
On 26/05/2015 14:41, Martijn Dashorst wrote:
> Done.
>
>
> On Tue, May 26, 2015 at 2:39 PM, Martijn Dashorst
> <ma...@gmail.com> wrote:
>> OK, then I'll commit and push.
>>
>> Martijn
>>
>> On Tue, May 26, 2015 at 2:37 PM, Martin Grigorov <mg...@apache.org> wrote:
>>> This looks handy!
>>> I'm +1 to have it in -core.
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>> https://twitter.com/mtgrigorov
>>>
>>> On Tue, May 26, 2015 at 3:33 PM, Martijn Dashorst <
>>> martijn.dashorst@gmail.com> wrote:
>>>
>>>> Using this class I was able to migrate our 7 invocations within a
>>>> couple of minutes.
>>>>
>>>> Martijn
>>>>
>>>>
>>>> On Tue, May 26, 2015 at 2:32 PM, Martijn Dashorst
>>>> <ma...@gmail.com> wrote:
>>>>> So instead of re-introducing the constructors, we could opt to provide
>>>>> a utility that makes it easy to refactor code to the new fluent API,
>>>>> and document its use in the migration guide.
>>>>>
>>>>> For example:
>>>>>
>>>>> https://gist.github.com/dashorst/5ae5ebcdedab9da34792
>>>>>
>>>>> We could create a wicket-migrate-7.x module to add more of these
>>>>> classes if necessary/useful. But for convenience they should be in
>>>>> core.
>>>>>
>>>>> Martijn
>>>>>
>>>>>
>>>>> On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
>>>>> <ma...@gmail.com> wrote:
>>>>>> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org>
>>>> wrote:
>>>>>>> By reverting the constructors from 6.x we will reintroduce
>>>>>>> https://issues.apache.org/jira/browse/WICKET-4972
>>>>>> In our 1.2M lines of code project, there are only 7 invocations of the
>>>>>> StringResourceModel constructor that I have to figure out, so this is
>>>>>> (for our company at least) a minor issue. I can imagine that
>>>>>> internationalized apps will have a bigger problem.
>>>>>>
>>>>>> I don't know how to handle WICKET-4972. The way it was in Wicket 7
>>>>>> broke silently for us, while the situation from Wicket 6.19 and prior
>>>>>> was working.
>>>>>>
>>>>>> Martijn
>>>>>
>>>>>
>>>>> --
>>>>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>>
>>>>
>>>> --
>>>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>>
>>
>>
>> --
>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>
>
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
Done.
On Tue, May 26, 2015 at 2:39 PM, Martijn Dashorst
<ma...@gmail.com> wrote:
> OK, then I'll commit and push.
>
> Martijn
>
> On Tue, May 26, 2015 at 2:37 PM, Martin Grigorov <mg...@apache.org> wrote:
>> This looks handy!
>> I'm +1 to have it in -core.
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>> https://twitter.com/mtgrigorov
>>
>> On Tue, May 26, 2015 at 3:33 PM, Martijn Dashorst <
>> martijn.dashorst@gmail.com> wrote:
>>
>>> Using this class I was able to migrate our 7 invocations within a
>>> couple of minutes.
>>>
>>> Martijn
>>>
>>>
>>> On Tue, May 26, 2015 at 2:32 PM, Martijn Dashorst
>>> <ma...@gmail.com> wrote:
>>> > So instead of re-introducing the constructors, we could opt to provide
>>> > a utility that makes it easy to refactor code to the new fluent API,
>>> > and document its use in the migration guide.
>>> >
>>> > For example:
>>> >
>>> > https://gist.github.com/dashorst/5ae5ebcdedab9da34792
>>> >
>>> > We could create a wicket-migrate-7.x module to add more of these
>>> > classes if necessary/useful. But for convenience they should be in
>>> > core.
>>> >
>>> > Martijn
>>> >
>>> >
>>> > On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
>>> > <ma...@gmail.com> wrote:
>>> >> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org>
>>> wrote:
>>> >>> By reverting the constructors from 6.x we will reintroduce
>>> >>> https://issues.apache.org/jira/browse/WICKET-4972
>>> >>
>>> >> In our 1.2M lines of code project, there are only 7 invocations of the
>>> >> StringResourceModel constructor that I have to figure out, so this is
>>> >> (for our company at least) a minor issue. I can imagine that
>>> >> internationalized apps will have a bigger problem.
>>> >>
>>> >> I don't know how to handle WICKET-4972. The way it was in Wicket 7
>>> >> broke silently for us, while the situation from Wicket 6.19 and prior
>>> >> was working.
>>> >>
>>> >> Martijn
>>> >
>>> >
>>> >
>>> > --
>>> > Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>
>>>
>>>
>>> --
>>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
OK, then I'll commit and push.
Martijn
On Tue, May 26, 2015 at 2:37 PM, Martin Grigorov <mg...@apache.org> wrote:
> This looks handy!
> I'm +1 to have it in -core.
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
> On Tue, May 26, 2015 at 3:33 PM, Martijn Dashorst <
> martijn.dashorst@gmail.com> wrote:
>
>> Using this class I was able to migrate our 7 invocations within a
>> couple of minutes.
>>
>> Martijn
>>
>>
>> On Tue, May 26, 2015 at 2:32 PM, Martijn Dashorst
>> <ma...@gmail.com> wrote:
>> > So instead of re-introducing the constructors, we could opt to provide
>> > a utility that makes it easy to refactor code to the new fluent API,
>> > and document its use in the migration guide.
>> >
>> > For example:
>> >
>> > https://gist.github.com/dashorst/5ae5ebcdedab9da34792
>> >
>> > We could create a wicket-migrate-7.x module to add more of these
>> > classes if necessary/useful. But for convenience they should be in
>> > core.
>> >
>> > Martijn
>> >
>> >
>> > On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
>> > <ma...@gmail.com> wrote:
>> >> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org>
>> wrote:
>> >>> By reverting the constructors from 6.x we will reintroduce
>> >>> https://issues.apache.org/jira/browse/WICKET-4972
>> >>
>> >> In our 1.2M lines of code project, there are only 7 invocations of the
>> >> StringResourceModel constructor that I have to figure out, so this is
>> >> (for our company at least) a minor issue. I can imagine that
>> >> internationalized apps will have a bigger problem.
>> >>
>> >> I don't know how to handle WICKET-4972. The way it was in Wicket 7
>> >> broke silently for us, while the situation from Wicket 6.19 and prior
>> >> was working.
>> >>
>> >> Martijn
>> >
>> >
>> >
>> > --
>> > Become a Wicket expert, learn from the best: http://wicketinaction.com
>>
>>
>>
>> --
>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
This looks handy!
I'm +1 to have it in -core.
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Tue, May 26, 2015 at 3:33 PM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:
> Using this class I was able to migrate our 7 invocations within a
> couple of minutes.
>
> Martijn
>
>
> On Tue, May 26, 2015 at 2:32 PM, Martijn Dashorst
> <ma...@gmail.com> wrote:
> > So instead of re-introducing the constructors, we could opt to provide
> > a utility that makes it easy to refactor code to the new fluent API,
> > and document its use in the migration guide.
> >
> > For example:
> >
> > https://gist.github.com/dashorst/5ae5ebcdedab9da34792
> >
> > We could create a wicket-migrate-7.x module to add more of these
> > classes if necessary/useful. But for convenience they should be in
> > core.
> >
> > Martijn
> >
> >
> > On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
> > <ma...@gmail.com> wrote:
> >> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org>
> wrote:
> >>> By reverting the constructors from 6.x we will reintroduce
> >>> https://issues.apache.org/jira/browse/WICKET-4972
> >>
> >> In our 1.2M lines of code project, there are only 7 invocations of the
> >> StringResourceModel constructor that I have to figure out, so this is
> >> (for our company at least) a minor issue. I can imagine that
> >> internationalized apps will have a bigger problem.
> >>
> >> I don't know how to handle WICKET-4972. The way it was in Wicket 7
> >> broke silently for us, while the situation from Wicket 6.19 and prior
> >> was working.
> >>
> >> Martijn
> >
> >
> >
> > --
> > Become a Wicket expert, learn from the best: http://wicketinaction.com
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
>
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
Using this class I was able to migrate our 7 invocations within a
couple of minutes.
Martijn
On Tue, May 26, 2015 at 2:32 PM, Martijn Dashorst
<ma...@gmail.com> wrote:
> So instead of re-introducing the constructors, we could opt to provide
> a utility that makes it easy to refactor code to the new fluent API,
> and document its use in the migration guide.
>
> For example:
>
> https://gist.github.com/dashorst/5ae5ebcdedab9da34792
>
> We could create a wicket-migrate-7.x module to add more of these
> classes if necessary/useful. But for convenience they should be in
> core.
>
> Martijn
>
>
> On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
> <ma...@gmail.com> wrote:
>> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org> wrote:
>>> By reverting the constructors from 6.x we will reintroduce
>>> https://issues.apache.org/jira/browse/WICKET-4972
>>
>> In our 1.2M lines of code project, there are only 7 invocations of the
>> StringResourceModel constructor that I have to figure out, so this is
>> (for our company at least) a minor issue. I can imagine that
>> internationalized apps will have a bigger problem.
>>
>> I don't know how to handle WICKET-4972. The way it was in Wicket 7
>> broke silently for us, while the situation from Wicket 6.19 and prior
>> was working.
>>
>> Martijn
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
So instead of re-introducing the constructors, we could opt to provide
a utility that makes it easy to refactor code to the new fluent API,
and document its use in the migration guide.
For example:
https://gist.github.com/dashorst/5ae5ebcdedab9da34792
We could create a wicket-migrate-7.x module to add more of these
classes if necessary/useful. But for convenience they should be in
core.
Martijn
On Tue, May 26, 2015 at 2:09 PM, Martijn Dashorst
<ma...@gmail.com> wrote:
> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org> wrote:
>> By reverting the constructors from 6.x we will reintroduce
>> https://issues.apache.org/jira/browse/WICKET-4972
>
> In our 1.2M lines of code project, there are only 7 invocations of the
> StringResourceModel constructor that I have to figure out, so this is
> (for our company at least) a minor issue. I can imagine that
> internationalized apps will have a bigger problem.
>
> I don't know how to handle WICKET-4972. The way it was in Wicket 7
> broke silently for us, while the situation from Wicket 6.19 and prior
> was working.
>
> Martijn
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
IMO compilation error + better explanation in the migration guide how to
fix the compilation error would be better than reverting to the 6.x
constructors.
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Tue, May 26, 2015 at 3:09 PM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:
> On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org>
> wrote:
> > By reverting the constructors from 6.x we will reintroduce
> > https://issues.apache.org/jira/browse/WICKET-4972
>
> In our 1.2M lines of code project, there are only 7 invocations of the
> StringResourceModel constructor that I have to figure out, so this is
> (for our company at least) a minor issue. I can imagine that
> internationalized apps will have a bigger problem.
>
> I don't know how to handle WICKET-4972. The way it was in Wicket 7
> broke silently for us, while the situation from Wicket 6.19 and prior
> was working.
>
> Martijn
>
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
On Tue, May 26, 2015 at 2:02 PM, Martin Grigorov <mg...@apache.org> wrote:
> By reverting the constructors from 6.x we will reintroduce
> https://issues.apache.org/jira/browse/WICKET-4972
In our 1.2M lines of code project, there are only 7 invocations of the
StringResourceModel constructor that I have to figure out, so this is
(for our company at least) a minor issue. I can imagine that
internationalized apps will have a bigger problem.
I don't know how to handle WICKET-4972. The way it was in Wicket 7
broke silently for us, while the situation from Wicket 6.19 and prior
was working.
Martijn
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
By reverting the constructors from 6.x we will reintroduce
https://issues.apache.org/jira/browse/WICKET-4972
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Tue, May 26, 2015 at 2:55 PM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:
> While trying to fix the resulting compile errors of this change, I
> found it hard to decipher what each parameter meant (which is why we
> opted going for the fluent interface in the first place)
>
> So take this example for instance:
>
> addColumn(new CustomPropertyColumn<Signaal>(
> new StringResourceModel(
> "studentnumber",
> null,
> "studentnumber",
> FooApp.get().getStudentTermUpperCase() + "number"
> ),
> new StringResourceModel(
> "studentnumber",
> null,
> "studentnumber",
> FooApp.get().getStudentTermUpperCase() + "nummer"
> ), "event.student.studentnumber")
> .setDefaultVisible(false));
>
> addColumn(new CustomPropertyColumn<Signaal>(
> new StringResourceModel(
> "student",
> null,
> "student",
> FooApp.get().getStudentTermUpperCase()
> ), new StringResourceModel(
> "student",
> null,
> "student",
> FooApp.get().getStudentTermUpperCase()
> ),
> "event.student.person.fullName")
> .setDefaultVisible(false));
>
> If you need to fix the compile error, there is no guidance left for
> figuring out which parameters are passed in (which was already a pain,
> but you could conceivably figure it out by navigating to the relevant
> constructor that Eclipse or IntelliJ was using. By breaking hard, the
> transition is (perhaps needlessly) hard.
>
> We could reïnstate the old constructors from 6.19.0 and mark them as
> deprecated with the hint to refactor them into the fluent interface.
> This would make transitioning (much [1]) easier.
>
> What do you guys think?
>
> Martijn
>
> [1] depending on the amount of internationalisation and usage of
> StringResourceModel, and whether you have crafted a wrapper around the
> beast.
>
>
>
>
>
>
>
> On Fri, May 15, 2015 at 2:10 PM, Martijn Dashorst
> <ma...@gmail.com> 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
> > <gu...@gmail.com> 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 <mg...@apache.org> 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 <
> >>> martijn.dashorst@gmail.com> 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
> >>>> <gu...@gmail.com> 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 <
> >>>> guillaume.smet@gmail.com>
> >>>>> 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 <
> mgrigorov@apache.org>
> >>>>>> 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 <
> >>>>>>> guillaume.smet@gmail.com>
> >>>>>>> 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
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
>
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
While trying to fix the resulting compile errors of this change, I
found it hard to decipher what each parameter meant (which is why we
opted going for the fluent interface in the first place)
So take this example for instance:
addColumn(new CustomPropertyColumn<Signaal>(
new StringResourceModel(
"studentnumber",
null,
"studentnumber",
FooApp.get().getStudentTermUpperCase() + "number"
),
new StringResourceModel(
"studentnumber",
null,
"studentnumber",
FooApp.get().getStudentTermUpperCase() + "nummer"
), "event.student.studentnumber")
.setDefaultVisible(false));
addColumn(new CustomPropertyColumn<Signaal>(
new StringResourceModel(
"student",
null,
"student",
FooApp.get().getStudentTermUpperCase()
), new StringResourceModel(
"student",
null,
"student",
FooApp.get().getStudentTermUpperCase()
),
"event.student.person.fullName")
.setDefaultVisible(false));
If you need to fix the compile error, there is no guidance left for
figuring out which parameters are passed in (which was already a pain,
but you could conceivably figure it out by navigating to the relevant
constructor that Eclipse or IntelliJ was using. By breaking hard, the
transition is (perhaps needlessly) hard.
We could reïnstate the old constructors from 6.19.0 and mark them as
deprecated with the hint to refactor them into the fluent interface.
This would make transitioning (much [1]) easier.
What do you guys think?
Martijn
[1] depending on the amount of internationalisation and usage of
StringResourceModel, and whether you have crafted a wrapper around the
beast.
On Fri, May 15, 2015 at 2:10 PM, Martijn Dashorst
<ma...@gmail.com> 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
> <gu...@gmail.com> 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 <mg...@apache.org> 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 <
>>> martijn.dashorst@gmail.com> 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
>>>> <gu...@gmail.com> 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 <
>>>> guillaume.smet@gmail.com>
>>>>> 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 <mg...@apache.org>
>>>>>> 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 <
>>>>>>> guillaume.smet@gmail.com>
>>>>>>> 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
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
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
<gu...@gmail.com> 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 <mg...@apache.org> 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 <
>> martijn.dashorst@gmail.com> 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
>>> <gu...@gmail.com> 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 <
>>> guillaume.smet@gmail.com>
>>>> 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 <mg...@apache.org>
>>>>> 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 <
>>>>>> guillaume.smet@gmail.com>
>>>>>> 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
Re: Wicket 7 and StringResourceModel
Posted by Guillaume Smet <gu...@gmail.com>.
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 <mg...@apache.org> 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 <
> martijn.dashorst@gmail.com> 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
>> <gu...@gmail.com> 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 <
>> guillaume.smet@gmail.com>
>>> 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 <mg...@apache.org>
>>>> 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 <
>>>>> guillaume.smet@gmail.com>
>>>>> 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
>>
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
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 <
martijn.dashorst@gmail.com> 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
> <gu...@gmail.com> 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 <
> guillaume.smet@gmail.com>
> > 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 <mg...@apache.org>
> >> 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 <
> >>> guillaume.smet@gmail.com>
> >>> 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
>
Re: Wicket 7 and StringResourceModel
Posted by Martijn Dashorst <ma...@gmail.com>.
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
<gu...@gmail.com> 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 <gu...@gmail.com>
> 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 <mg...@apache.org>
>> 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 <
>>> guillaume.smet@gmail.com>
>>> 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
Re: Wicket 7 and StringResourceModel
Posted by Guillaume Smet <gu...@gmail.com>.
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 <gu...@gmail.com>
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 <mg...@apache.org>
> 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 <
>> guillaume.smet@gmail.com>
>> 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
>> >
>>
>
>
Re: Wicket 7 and StringResourceModel
Posted by Guillaume Smet <gu...@gmail.com>.
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 <mg...@apache.org>
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 <guillaume.smet@gmail.com
> >
> 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
> >
>
Re: Wicket 7 and StringResourceModel
Posted by Martin Grigorov <mg...@apache.org>.
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 <gu...@gmail.com>
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
>