Removing some final modifiers from Wicket

[Michael Bohn <mi...@freiheit.com>]

The pull request https://github.com/apache/wicket/pull/136 advocates
to remove some final modifiers from Wicket methods to allow them
to be overwritten.

The changes are:
Localizer.getStringIgnoreSettings
Component.addStateChange
MarkupContainer.renderAll

Removing final from Localizer.getStringIgnoreSettings is probably
undisputable. For the other two methods we have the following
reasons for removing the modifier (we patched Wicket in order to overwrite
them, but would see the change rather in
the wicket repo itself):

renderAll was overwritten by us to add some performance profiling,
something like this:
<startPerfProfiling>
super.renderAll()
<stopPerfProfiling>

addStateChange() was overwritten to prevent the increment of a page version
number, since we manipulated the component hierarchy after rendering, but
before the page gets stored. This is a very deep manipulation of Wicket, we
are fully aware of that, but it was essential for our high traffic scenario.

If you don't accept the pull request fully, I will split the request to
isolate the changes that can be merged.

Best regards,
Michael

Re: Removing some final modifiers from Wicket

[Martin Grigorov <mg...@apache.org>]

Hi,

Here is my vote/opinion:


On Tue, Aug 18, 2015 at 8:46 PM, Michael Bohn <mi...@freiheit.com>
wrote:

> The pull request https://github.com/apache/wicket/pull/136 advocates
> to remove some final modifiers from Wicket methods to allow them
> to be overwritten.
>
> The changes are:
> Localizer.getStringIgnoreSettings
> Component.addStateChange
> MarkupContainer.renderAll
>
> Removing final from Localizer.getStringIgnoreSettings is probably
>

+1


> undisputable. For the other two methods we have the following
> reasons for removing the modifier (we patched Wicket in order to overwrite
> them, but would see the change rather in
> the wicket repo itself):
>
> renderAll was overwritten by us to add some performance profiling,
> something like this:
> <startPerfProfiling>
> super.renderAll()
> <stopPerfProfiling>
>

-1


>
> addStateChange() was overwritten to prevent the increment of a page version
> number, since we manipulated the component hierarchy after rendering, but
> before the page gets stored. This is a very deep manipulation of Wicket, we
> are fully aware of that, but it was essential for our high traffic
> scenario.
>

-1


>
> If you don't accept the pull request fully, I will split the request to
> isolate the changes that can be merged.
>
> Best regards,
> Michael
>