If the tests run, then I'd say stick with it. Either way is not
perfect, and this seems like a good solution. Documentation is key
here!
Please also update the 1.2 upgrade docs.
Martijn
On 1/18/06, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
> just fixed the examples. the only one still failing in eclipse is the
> guestbook and that is only because it gets run twice (on second run it is
> not in a clean state so the test fails).
>
> so im -1 on rolling this back. and Juergen is +1. what the hell does
> everyone else think?
>
> -Igor
>
>
>
> On 1/16/06, Juergen Donnerstag <[EMAIL PROTECTED]> wrote:
> > Currently 30% of the wicket examples are failing because of it.
> > +1
> >
> > Juergen
> >
> > On 1/15/06, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
> > > the only thing that needs to be reverted are the few places i changed
> the
> > > calls to the pagefactory. should be quick and easy.
> > >
> > > so everyone for reverting?
> > >
> > > -Igor
> > >
> > >
> > > On 1/15/06, Juergen Donnerstag < [EMAIL PROTECTED]> wrote:
> > > >
> > > > Some jwebunit tests in wicket-examples are currently failing due to
> > > > the ipagefactory change. IMO we shouldn't wait to long to revert
> > > > everything back. We keep on making changes (like Jon javadoc fixes
> > > > etc.) and it'll be more difficult every day.
> > > >
> > > > Juergen
> > > >
> > > > On 1/15/06, Juergen Donnerstag <[EMAIL PROTECTED] > wrote:
> > > > > I'm for 2 methods in the factory. The one without params param is
> jut
> > > > > for convinience, that is right, but I don't think this is bad.
> > > > > And I think we should always check for the param constructor first.
> If
> > > > > present, call it. Else call the default constructor.
> > > > >
> > > > > Juergen
> > > > >
> > > > > On 1/14/06, Johan Compagner < [EMAIL PROTECTED]> wrote:
> > > > > > i thought that was the change that was requested to Not do that?
> > > > > >
> > > > > > So then we first have to decide what we want or better said what
> > > developers
> > > > > > expect..
> > > > > > i think i am +1 for checking the params map on size.
> > > > > >
> > > > > > Because if you have this:
> > > > > >
> > > > > > /wicketapp/bookmarkablepage
> > > > > > or
> > > > > > /wicketapp/bookmarkablepage/x/5/y/7
> > > > > >
> > > > > > and that page has a default and a params constructor? What is then
> the
> > > > > > feeling what the first should do?
> > > > > > (my feeling says call default that is how it in java also works)
> > > > > >
> > > > > > johan
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 1/14/06, Eelco Hillenius < [EMAIL PROTECTED]> wrote:
> > > > > > > I don't think that makes things easier. Why not ALLWAYS call the
> > > > > > > constructor with page parameters unless it isn't there in which
> case
> > > > > > > we call the default constructor? That's just a very simple rule
> > > > > > > instead of an algoritm, and thus is less error prone.
> > > > > > >
> > > > > > > Eelco
> > > > > > >
> > > > > > >
> > > > > > > On 1/14/06, Johan Compagner < [EMAIL PROTECTED] > wrote:
> > > > > > > > one method is fine.
> > > > > > > > But i think the complete flow has to be something like this
> > > > > > > >
> > > > > > > > boolean hasDefaultConstructor = xxx;
> > > > > > > > boolean hasParamsConstructor = xxx;
> > > > > > > >
> > > > > > > > boolean pageParams = params == null || params.size () == 0;
> > > > > > > >
> > > > > > > > if( (pageParams)
> > > > > > > > {
> > > > > > > > if(hasParamsConstructor)
> > > > > > > > {
> > > > > > > > // call params constructor
> > > > > > > > }
> > > > > > > > else if(hasDefaultConstructor)
> > > > > > > > {
> > > > > > > > // call default
> > > > > > > > }
> > > > > > > > else { throw exception}
> > > > > > > >
> > > > > > > > }
> > > > > > > > else
> > > > > > > > {
> > > > > > > > if(hasDefaultConstructor)
> > > > > > > > {
> > > > > > > > // call default
> > > > > > > > }
> > > > > > > > else if(hasParamsConstructor) {
> > > > > > > > // call params constructor with new PageParams() empty map;
> > > > > > > > }
> > > > > > > > else { throw exception}
> > > > > > > > }
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On 1/14/06, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
> > > > > > > > >
> > > > > > > > > What I don't like about IPageFactory is that it has two
> methods
> > > > > > > > > instead of just one (the one with class and page
> parameters).
> > > That
> > > > > > > > > page parameter may be null - part of the contract, thus
> fine.
> > > And
> > > > > > > > > constructors with the page parameter should allways precede
> the
> > > > > > > > > default constructor, even if the page parameters argument is
> > > null.
> > > > > > > > > Only when there is no pageparameters constructor should the
> > > default
> > > > > > > > > constructor be called. Wouldn't this be a much clearer
> contract?
> > > > > > > > >
> > > > > > > > > Eelco
> > > > > > > > >
> > > > > > > > > On 1/14/06, Igor Vaynberg < [EMAIL PROTECTED] > wrote:
> > > > > > > > > > hey guys, while helping pepone pepone on the user list ive
> > > come
> > > > > > across a
> > > > > > > > > > bug. most of the time when we instantiate a page we do it
> > > using a
> > > > > > wrong
> > > > > > > > call
> > > > > > > > > > to the pagefactory. most of the time we use newPage(class,
> > > > > > PageParams)
> > > > > > > > even
> > > > > > > > > > though page params are empty and so the newPage(Class)
> should
> > > be
> > > > > > used
> > > > > > > > > > instead which invokes the default constructor.
> > > > > > > > > >
> > > > > > > > > > i went through all the places i could find and basically
> did
> > > this
> > > > > > > > > > if (pageparams.size ()==0) { page=newPage(class); } else {
> > > > > > > > > > page=newPage(class, pageparams); }
> > > > > > > > > >
> > > > > > > > > > and then a lot of tests broke because they depend on
> wicket
> > > calling
> > > > > > the
> > > > > > > > > > wrong constructor! so i had to go and fix all those. i
> would
> > > say 90%
> > > > > > of
> > > > > > > > all
> > > > > > > > > > test pages were expecting to be instantiated with the
> wrong
> > > > > > constructor,
> > > > > > > > > > probably because they were copied and pasted most of the
> time.
> > > > > > > > > >
> > > > > > > > > > while doing all this and discussing it with eelco we
> agreed
> > > that it
> > > > > > > > might be
> > > > > > > > > > better to change ipagefactory to only have one
> constructor:
> > > (class,
> > > > > > > > > > pageparams) and allow pagaparams to be null. the rule
> would be
> > > > > > simple:
> > > > > > > > if
> > > > > > > > > > page params==null||pageparams.size()==0
> use
> > > the
> > > > > > default
> > > > > > > > > > constructor, otherwise use the pageparams one.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > what do you think?
> > > > > > > > > >
> > > > > > > > > > -Igor
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > >
> > > -------------------------------------------------------
> > > > > > > > > This SF.net email is sponsored by: Splunk Inc. Do you grep
> > > through log
> > > > > > > > files
> > > > > > > > > for problems? Stop! Download the new AJAX search engine
> that
> > > makes
> > > > > > > > > searching your log files as easy as surfing the web.
> DOWNLOAD
> > > > > > SPLUNK!
> > > > > > > > >
> > > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > > > > > >
> _______________________________________________
> > > > > > > > > Wicket-develop mailing list
> > > > > > > > > Wicket-develop@lists.sourceforge.net
> > > > > > > > >
> > > > > > > >
> > > > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > -------------------------------------------------------
> > > > > > > This SF.net email is sponsored by: Splunk Inc. Do you grep
> through
> > > log
> > > > > > files
> > > > > > > for problems? Stop! Download the new AJAX search engine that
> makes
> > > > > > > searching your log files as easy as surfing the web. DOWNLOAD
> > > SPLUNK!
> > > > > > >
> http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > > > > _______________________________________________
> > > > > > > Wicket-develop mailing list
> > > > > > > Wicket-develop@lists.sourceforge.net
> > > > > > >
> > > > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> > > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> -------------------------------------------------------
> > > > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> > > files
> > > > for problems? Stop! Download the new AJAX search engine that makes
> > > > searching your log files as easy as surfing the web. DOWNLOAD
> SPLUNK!
> > > > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > _______________________________________________
> > > > Wicket-develop mailing list
> > > > Wicket-develop@lists.sourceforge.net
> > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> > > >
> > >
> > >
> >
> >
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> > for problems? Stop! Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > _______________________________________________
> > Wicket-develop mailing list
> > Wicket-develop@lists.sourceforge.net
> >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
>
>
--
Living a wicket life...
Martijn Dashorst - http://www.jroller.com/page/dashorst
Wicket 1.1 is out: http://wicket.sourceforge.net/wicket-1.1
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid3432&bid#0486&dat1642
_______________________________________________
Wicket-develop mailing list
Wicket-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-develop
