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

Reply via email to