Carl-Eric, could you please re-format the related classes in wicket-6.x branch so all noise like https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380 disappear ? Just open the same classes in wicket-6.x branch, format them, commit+push. Cherry-pick in master.
I've added a comment on one of the commits in GitHub but so far I don't received a notification about it. Did you receive a notification about https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068 ? Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov On Wed, Aug 20, 2014 at 2:31 PM, Martin Grigorov <[email protected]> wrote: > to review: > https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677 > > Martin Grigorov > Wicket Training and Consulting > https://twitter.com/mtgrigorov > > > On Mon, Aug 18, 2014 at 3:18 PM, Carl-Eric Menzel <[email protected]> > wrote: > >> On Mon, 18 Aug 2014 13:30:03 +0200 >> Martin Grigorov <[email protected]> wrote: >> >> > I don't like the whole idea of re-adding component. >> > Wizard is the only component in Wicket distro which fails >> > the >> > org.apache.wicket.core.util.objects.checker.OrphanComponentChecker. I >> > think only the data (i.e. the models) should be kept around and the >> > components should be re-created for every view of that data when this >> > is needed. For me Wizard component is a bad practice. >> >> I'm not convinced it is entirely bad, but I can see that it is at least >> not optimal. >> >> However, Wicket does not prohibit it and so I think we should make sure >> it doesn't break in surprising ways. >> >> That said, I don't want to conflate this with the problems of Wizard >> too much, because I still think we should have a complement to >> onRemove, whether we're looking at Wizard or not, just to be consistent. >> >> I had a chat with Martin on IRC, he had a very good idea on how to >> simplify onAddToPage to a more clearly defined onReAdd. Thank you >> Martin! >> >> I've pushed an update including tests to the WICKET-5677 branch. >> >> The much simpler implementation is now not much more than an else branch >> in Component#fireInitialize. If the component is already initialized, >> it doesn't call onInitialize, but rather onReAdd. This way we have a >> clear distinction between the two callbacks and we don't need a second >> tree traversal, since this just uses the one that is there for >> onInitialize already. >> >> Martin and everyone else, what do you think? >> >> Carl-Eric >> > >
