A sample for logging (issue641801)

10 views
Skip to first unread message

unn...@google.com

unread,
Jun 17, 2010, 8:20:09 PM6/17/10
to fre...@google.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com
Reviewers: fredsa,

Description:
A sample for logging


Please review this at http://gwt-code-reviews.appspot.com/641801/show

Affected files:
A eclipse/samples/LogExample/LogExample.gwtc.launch
A eclipse/samples/LogExample/LogExample.launch
A eclipse/samples/LogExample/war/LogExample.css
A eclipse/samples/LogExample/war/LogExample.html
A samples/logexample/build.xml
A samples/logexample/src/com/google/gwt/sample/logexample/COPYING
A
samples/logexample/src/com/google/gwt/sample/logexample/LogExample.gwt.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/LogExample.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/LogExample.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java
A
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.ui.xml
A
samples/logexample/src/com/google/gwt/sample/logexample/server/LoggingServiceImpl.java
A
samples/logexample/src/com/google/gwt/sample/logexample/shared/LoggingService.java
A
samples/logexample/src/com/google/gwt/sample/logexample/shared/LoggingServiceAsync.java
A
samples/logexample/src/com/google/gwt/sample/logexample/shared/SharedLoggingLibrary.java
A samples/logexample/war/LogExample.css
A samples/logexample/war/LogExample.html


fre...@google.com

unread,
Jun 18, 2010, 1:21:55 PM6/18/10
to unn...@google.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com
LGTM

nit: there are a few *.java files which have inconsistent use of blank
lines


http://gwt-code-reviews.appspot.com/641801/diff/1/9
File
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/9#newcode30
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.java:30:
* WRITE ME
Write me :)

http://gwt-code-reviews.appspot.com/641801/diff/1/10
File
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.ui.xml
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/10#newcode5
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.ui.xml:5:
multiple add calls can be used by the HasWidgetsLogHandler. Here we've
add -> add()

http://gwt-code-reviews.appspot.com/641801/diff/1/11
File
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/11#newcode39
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java:39:
* WRITE ME
Write me

http://gwt-code-reviews.appspot.com/641801/diff/1/11#newcode90
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java:90:
setupHandler("not implemented", remoteCheckbox);
Should this line just be commented out instead?

http://gwt-code-reviews.appspot.com/641801/diff/1/15
File
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/15#newcode29
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.java:29:
* WRITE ME
write me

http://gwt-code-reviews.appspot.com/641801/diff/1/17
File
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/17#newcode34
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.java:34:
* WRITE ME
write me

http://gwt-code-reviews.appspot.com/641801/diff/1/19
File
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/19#newcode34
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java:34:
* WRITE ME
write me

http://gwt-code-reviews.appspot.com/641801/diff/1/19#newcode53
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java:53:
SharedLoggingLibrary.logUsingSharedLibrary(Level.SEVERE, "FOO");
Change "FOO" to something more descriptive

http://gwt-code-reviews.appspot.com/641801/diff/1/24
File
samples/logexample/src/com/google/gwt/sample/logexample/shared/SharedLoggingLibrary.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/24#newcode23
samples/logexample/src/com/google/gwt/sample/logexample/shared/SharedLoggingLibrary.java:23:
* WRITE ME
write me

http://gwt-code-reviews.appspot.com/641801/show

unn...@google.com

unread,
Jun 18, 2010, 6:10:49 PM6/18/10
to fre...@google.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com

unn...@google.com

unread,
Jun 18, 2010, 6:10:54 PM6/18/10
to fre...@google.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com

http://gwt-code-reviews.appspot.com/641801/diff/1/9
File
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/9#newcode30
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.java:30:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> Write me :)

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/10
File
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.ui.xml
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/10#newcode5
samples/logexample/src/com/google/gwt/sample/logexample/client/CustomLogArea.ui.xml:5:
multiple add calls can be used by the HasWidgetsLogHandler. Here we've

On 2010/06/18 17:21:55, fredsa wrote:
> add -> add()

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/11
File
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/11#newcode39
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java:39:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> Write me

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/11#newcode90
samples/logexample/src/com/google/gwt/sample/logexample/client/HandlerController.java:90:
setupHandler("not implemented", remoteCheckbox);

On 2010/06/18 17:21:55, fredsa wrote:
> Should this line just be commented out instead?

I wanted there to be one disabled handler on the page, but you're right
- this one is confusing. I removed it, and disable the firebug handler
by default instead in the gwt.xml file.

http://gwt-code-reviews.appspot.com/641801/diff/1/15
File
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/15#newcode29
samples/logexample/src/com/google/gwt/sample/logexample/client/LoggerController.java:29:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> write me

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/17
File
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/17#newcode34
samples/logexample/src/com/google/gwt/sample/logexample/client/OneLoggerController.java:34:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> write me

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/19
File
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/19#newcode34
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java:34:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> write me

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/19#newcode53
samples/logexample/src/com/google/gwt/sample/logexample/client/ServerLoggingArea.java:53:
SharedLoggingLibrary.logUsingSharedLibrary(Level.SEVERE, "FOO");

On 2010/06/18 17:21:55, fredsa wrote:
> Change "FOO" to something more descriptive

Done.

http://gwt-code-reviews.appspot.com/641801/diff/1/24
File
samples/logexample/src/com/google/gwt/sample/logexample/shared/SharedLoggingLibrary.java
(right):

http://gwt-code-reviews.appspot.com/641801/diff/1/24#newcode23
samples/logexample/src/com/google/gwt/sample/logexample/shared/SharedLoggingLibrary.java:23:
* WRITE ME

On 2010/06/18 17:21:55, fredsa wrote:
> write me

Done.

http://gwt-code-reviews.appspot.com/641801/show

markovu...@gmail.com

unread,
Jun 21, 2010, 5:49:28 AM6/21/10
to unn...@google.com, fre...@google.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com
I think that .project, .classpath and .checkstyle files should be
included. All the other samples have those files imported. It is
possible to create the eclipse project from sources but I think it would
be much more convenient if the project was already created for the user.
I also think that this should be added for the sake of consistency with
other projects.

http://gwt-code-reviews.appspot.com/641801/show

Unnur Gretarsdottir

unread,
Jun 21, 2010, 12:06:31 PM6/21/10
to unn...@google.com, fre...@google.com, markovu...@gmail.com, google-web-tool...@googlegroups.com, re...@gwt-code-reviews.appspotmail.com
Sure - that sounds reasonable - I'll add those today.
Reply all
Reply to author
Forward
0 new messages