On Thu, Aug 22, 2013 at 6:30 PM, Igor Vaynberg <igor.vaynb...@gmail.com> wrote: > i added a bunch of comments in the gist. not sure why we couldnt just do it > here in place....
We can sure discuss it here, like I told you on ##wicket. I just don't think posting a 200+ line document that still needs revisions to a mailing list is appropriate. But the discussion can most certainly happen here. I've copied your comments into this message. Igor Vaynberg commented: https://gist.github.com/dashorst/6308833/#comment-891779 > if we were to implement 2.2 (field injection) this would have to be > done from the Component's constructor, so we would spread creation > and initialization of parameters across two places: component and > page factory - this sucks. The logic can be centralized, calling the logic is from two locations. Not too big a problem IMO. I'd rather talk about what we can or can't achieve with allowing field injection. I think it is logical to follow JAX-RS closely in what they provide, unless it is too expensive/impossible to implement. > also, it is rarely these things are useful as fields because we dont > usually want them serialized. most of the time we pull a value, wrap > it in an LDM and store that as a field. Currently the page parameters are already serialized with the page (IIRC) so this is not something new. In fact it makes it more visible *what* is serialized with the page, as I don't intend to store everything that was provided with the request with the page. After the request has completed we can either null the values or keep them. Igor Vaynberg commented: https://gist.github.com/dashorst/6308833/#comment-891799 > 2.3 Custom type parameters > > it is not enough to simply say you need a constructor that takes a > String. we need a converter system. for example, we often pull ids > out of PageParameters and load entities. we cannot have entities take > a string parameter and then reload itself with data. Wicket already has a converter system. This proposal is intended to be a replacement for PageParameters, not a full blown magic eight ball that will pull entities out of the database. Again I strive to implement something akin JAX-RS where they only provide the basic conversions, not full out-of-the-box-entity-loading. > whether or not we can/should reuse the existing converter > infrastructure we have should also be discussed. The current IConverters are pretty much what is required to do the basic conversions. It also provides good reuse of existing code. Why would we choose not to use it in case there is a converter registered for a parameter? Igor Vaynberg commented: https://gist.github.com/dashorst/6308833/#comment-891800 > From RFC-0001#3.2: > > // register all pages in the package and its child packages > > mountPackage("com.example.web.pages"); > > that will basically require a package scan, so not sure why we are > still discussing whether or not we need one in 3.3... Yes, that is why I stated in 3.3 that this is probably not necessary if we provide mountPackage: > From RFC-0001#3.3: > | NB: This is up for debate: mountPackage is simple enough to not > | have to implement a separate scan. Igor Vaynberg commented: https://gist.github.com/dashorst/6308833/#comment-891804 > we should keep the existing PageParameters infrastructure unchanged. > i do not want to migrate all the pages that use PageParameters when I > upgrade I propose this change for a new major release of Wicket, i.e. 7 or 8 (I'd rather do so in 7). We often don't shy away of introducing big changes if it is for the better. It is my intention to get this RFC to such a level you want to upgrade to Wicket 7 just to be able to rewrite your parameter handling. While I agree that the pain of upgrading will be hard, the benefit should really outweigh the cost of doing so in terms of maintainability, readability and understandability of your application code. That said, I can imagine leaving a sliver of PageParameters intact, to account for usecases we were unable to address in this RFC. I'll add a section on migration to the document on how I envision such a thing. > in a few instances page parameters are dynamic and i do not want > to/cannot list all possible ones in annotations You have to specify them now already, and if we implement the @BeanParam in combination with a List<String> I surmise you can do everything you currently can. Martijn
