[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
LGTM On Mon, Aug 2, 2010 at 11:26 AM, wrote: > On 2010/08/02 18:14:47, unnurg wrote: > >> On 2010/08/02 18:10:21, unnurg wrote: >> > On 2010/08/02 17:55:43, unnurg wrote: >> > > http://gwt-code-reviews.appspot.com/721803/diff/1/7 >> > > File >> > > >> > >> > > > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java > >> > > (right): >> > > >> > > http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 >> > > >> > >> > > > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: > >> > > // Does not work - ID is based on the user >> > > On 2010/08/02 17:32:43, Ray Ryan wrote: >> > > > This should be JavaDoc (and a little bit more descriptive) >> > > >> > > Done. >> > >> > Spoke to Ray offline and the objection he had was to having the >> > getUserInformationRequest() function in the RequestFactory >> > interface, rather > >> > than having it explicitly added in the FooRequestFactory interface. >> > This was > >> > easy to change, so just went ahead and did it in this change - patch >> > being > >> > uploaded now... >> > > Actually - that may have been a lie - debugging now... >> > Nope - it was the truth - it seems the issue I was seeing was due to old > generated files somehow - a clean build seems to work fine. > > > > > > > http://gwt-code-reviews.appspot.com/721803/show > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
On 2010/08/02 18:14:47, unnurg wrote: On 2010/08/02 18:10:21, unnurg wrote: > On 2010/08/02 17:55:43, unnurg wrote: > > http://gwt-code-reviews.appspot.com/721803/diff/1/7 > > File > > > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java > > (right): > > > > http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 > > > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: > > // Does not work - ID is based on the user > > On 2010/08/02 17:32:43, Ray Ryan wrote: > > > This should be JavaDoc (and a little bit more descriptive) > > > > Done. > > Spoke to Ray offline and the objection he had was to having the > getUserInformationRequest() function in the RequestFactory interface, rather > than having it explicitly added in the FooRequestFactory interface. This was > easy to change, so just went ahead and did it in this change - patch being > uploaded now... Actually - that may have been a lie - debugging now... Nope - it was the truth - it seems the issue I was seeing was due to old generated files somehow - a clean build seems to work fine. http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
On 2010/08/02 18:10:21, unnurg wrote: On 2010/08/02 17:55:43, unnurg wrote: > http://gwt-code-reviews.appspot.com/721803/diff/1/7 > File > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java > (right): > > http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 > bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: > // Does not work - ID is based on the user > On 2010/08/02 17:32:43, Ray Ryan wrote: > > This should be JavaDoc (and a little bit more descriptive) > > Done. Spoke to Ray offline and the objection he had was to having the getUserInformationRequest() function in the RequestFactory interface, rather than having it explicitly added in the FooRequestFactory interface. This was easy to change, so just went ahead and did it in this change - patch being uploaded now... Actually - that may have been a lie - debugging now... http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
On 2010/08/02 17:55:43, unnurg wrote: http://gwt-code-reviews.appspot.com/721803/diff/1/7 File bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java (right): http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: // Does not work - ID is based on the user On 2010/08/02 17:32:43, Ray Ryan wrote: > This should be JavaDoc (and a little bit more descriptive) Done. Spoke to Ray offline and the objection he had was to having the getUserInformationRequest() function in the RequestFactory interface, rather than having it explicitly added in the FooRequestFactory interface. This was easy to change, so just went ahead and did it in this change - patch being uploaded now... http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
http://gwt-code-reviews.appspot.com/721803/diff/1/7 File bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java (right): http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: // Does not work - ID is based on the user On 2010/08/02 17:32:43, Ray Ryan wrote: This should be JavaDoc (and a little bit more descriptive) Done. http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
On 2010/08/02 17:32:43, Ray Ryan wrote: LGTM The problem with this approach is that it's so hugely prescriptive — there is one and only one way to deal with user auth in RequestFactory. I think it will do to get the conversation started in M3, but we'll need to find something more flexible before we ship. Can you tell me more about what you mean here? This CL shouldn't actually have changed anything as far as the ability to configure the User services - the only change is that the base implementation is now checked into GWT rather than generated. In order to use a different authentication method, you would still extend the UserInformation class, and use the web.xml file to indicate that the app should use your class instead. You still have the ability to write your own widgets that use getUserInformationRequest (rather than (or in addition to) using the LoginWidget. Am I missing something else that you had in mind? http://gwt-code-reviews.appspot.com/721803/diff/1/7 File bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java (right): http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: // Does not work - ID is based on the user This should be JavaDoc (and a little bit more descriptive) Done http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change the way we include the User Auth files, keeping more of them (issue721803)
LGTM The problem with this approach is that it's so hugely prescriptive — there is one and only one way to deal with user auth in RequestFactory. I think it will do to get the conversation started in M3, but we'll need to find something more flexible before we ship. http://gwt-code-reviews.appspot.com/721803/diff/1/7 File bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java (right): http://gwt-code-reviews.appspot.com/721803/diff/1/7#newcode72 bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java:72: // Does not work - ID is based on the user This should be JavaDoc (and a little bit more descriptive) http://gwt-code-reviews.appspot.com/721803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
