https://issues.apache.org/jira/browse/WICKET-5889
On Tue, Apr 21, 2015 at 3:02 PM, Sebastien <[email protected]> wrote: > Hi Martin, > > Here we are: > > https://github.com/apache/wicket/commit/67dceb872015b9b664a811b456aee4aec6bb1303 > > Btw, squashing commits that have already been pushed is not... very > recommended! (I know it now ;) ) > > I will open the jira asap... > > Best regards, > Sebastien. > > > > On Tue, Apr 21, 2015 at 11:00 AM, Martin Grigorov <[email protected]> > wrote: > >> On Tue, Apr 21, 2015 at 11:50 AM, Sebastien <[email protected]> wrote: >> >> > Hi Martin, >> > >> > >> > 1) How did you push into https://github.com/apache/wicket without >> pushing >> > > at Apache Git repo? :-) >> > > I always thought that this repo is a mirror of Apache Git repo and >> > > read-only for anything else. >> > > >> > >> > > Actually I did push on my own fork/branch, but the commit is "visible" >> > from the apache repo. >> > For instance, if you review ("compare") your commits using [1] you are >> > automatically redirected to apache repo and then underlying commits >> somehow >> > looks like there are in apache repo... >> > >> >> Magic! >> >> >> > >> > [1] https://github.com/sebfz1/wicket/compare/master-sbriquet >> > >> > >> > > >> > > 2) Please create a Pull Request so we can see all commit changes in >> the >> > > same compare UI view >> > > >> > >> > I will try a rebase -i beforehand, it should lead to the same result for >> > reviewing without having to open a PR for now. This is because the first >> > commit is a "reformat" using wicket-eclipse-settings... >> > >> >> Yes, squashing will work too. >> >> >> > >> > >> > > >> > > 3) The call to visit.dontGoDeeper() at >> > > >> > > >> > >> https://github.com/apache/wicket/blob/cd3b92346b33b38c451434a1cabb7cf79d7188f0/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java#L1013 >> > > looks to me that Wicket won't work as expected for several layers >> > nesting. >> > > >> > >> > > To me, it makes sense: if the form "is enabled" or "is visible", we >> will >> > continue visiting children (the return statement) otherwise we stop >> > propagation because the inner form of a "non visible" or "not enabled" >> form >> > shouldn't be processed... >> > >> >> Oh. I've missed the 'return;' in the success case. >> >> >> > >> > >> > > The test you extend seems to deal with outer/middle/inner so I must be >> > > missing something. >> > > While you are "in the zone" - could you explain what happens? >> > > >> > >> > Are you referring to the whole testcases in the file, my testcase or >> > something else in particular ? >> > I prefer to ask you before answering, to avoid giving a long and >> headache >> > answer! ;) >> > >> >> The 'return;' above explains it. No need to answer! :-) >> >> >> > >> > >> > > >> > > 4) Please change the methods to 'protected' for master branch. There >> is >> > no >> > > reason to keep them public as Carl-Eric explained. >> > > >> > > >> > Doing it right now... >> > >> > Additional points: >> > >> > 5) Changing getFlag(FLAG_SUBMITTED) to isSubmitted(). Not sure how I >> missed >> > that... >> > >> > 6) Should I open a JIRA? >> > >> >> Oui! >> >> >> > >> > Thanks and best regards, >> > Sebastien. >> > >> > >
