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. > > >
