Thanks for the review. I will commit it as-is and we can make further changes as you use this code with HTMLUnit.
http://gwt-code-reviews.appspot.com/51835/diff/4011/4014 File dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java (right): http://gwt-code-reviews.appspot.com/51835/diff/4011/4014#newcode712 Line 712: DataInputStream stream = channel.getStreamFromOtherSide(); On 2009/08/07 01:19:52, amitmanjhi wrote: > Previous code used 'final' and there are other places in the code where 'final' > is used. Yes, Kelly likes to have final anywhere it can be. However, the rest of our code base is to only use final where it is particularly useful or required, such as instance fields and variables referenced by an anonymous class. So, I have been changing them to match the rest of our code when I edit those classes. http://gwt-code-reviews.appspot.com/51835/diff/4011/4014#newcode754 Line 754: writeUntaggedString(stream, methodName); On 2009/08/07 01:19:52, amitmanjhi wrote: > why are the method names not symmetric? readUtf8String vs. writeUntaggedString Kelly wrote the initial Messages classes and they have been modified from there. There is a bit of asymmetry about whether values are tagged or not, but it could be better represented -- perhaps a static helper class for tagged values. http://gwt-code-reviews.appspot.com/51835/diff/4011/4014#newcode1026 Line 1026: On 2009/08/07 01:19:52, amitmanjhi wrote: > For symmetry and clarity, why not have a writeMessageType method that takes care > of writing the tag? The write method in each method subclass can then just take > care of writing everything else. > This refactoring does not need to be done immediately. Ok. http://gwt-code-reviews.appspot.com/51835 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
