Demo refresh

Nils Loodin nils.loodin at oracle.com
Thu Apr 21 06:43:11 PDT 2011


Key issue is now resolved, and this can be looked at (by anyone!) here:
http://cr.openjdk.java.net/~nloodin/7029383/webrev.02/

So there's two things, as I said:
1. Expand imports or leave them alone

2. Keep jpda demo as is / keep it with these "light changes" / remove 
completely

Opinions?

I guess if there aren't any and I had my way I'd say that I'd like to
expand imports and remove the com/sun/tools/example/debug directory 
(where the code for the jpda demo resides)

/Nisse

On 04/18/2011 08:13 PM, Kelly O'Hair wrote:
> Hi Nils.
>
> This is fantastic, these demos needed some attention.
>
> You should scp the webrev to cr.openjdk.java.net 
> <http://cr.openjdk.java.net>, e.g.
>    scp -r 7029383/webrev.01/ nloodin at cr.openjdk.java.net 
> <mailto:nloodin at cr.openjdk.java.net>:
> Then anyone can view it with:
> http://cr.openjdk.java.net/~nloodin/webrev.01/ 
> <http://cr.openjdk.java.net/%7Enloodin/webrev.01/>
>
> My comments:
>
> src/share/classes/com/sun/tools/example/debug/bdi/ChildSession.java
>   If the quit methods are to be deleted, we should just delete the 
> lines, not comment out the methods.
> src/share/classes/com/sun/tools/example/debug/bdi/SourceNameReferenceTypeSpec.java
>   Dito on commented out code. I'd rather see it deleted, maybe leave 
> just a plain comment that
>   certain exceptions are just not thrown anymore.
> src/share/classes/com/sun/tools/example/debug/expr/ExpressionParser.java
>   What a yucky file. :^(  I'm not sure your changes have improved this 
> file, granted it may need
>   some more major surgery. The long lines make it very hard to review 
> the changes side by side.
>   You may need to look at this file again, the indentation seems 
> messed up in places.
> src/share/demo/nio/zipfs/Demo.java
>   Not sure I understand the comments on the getString()/getBytes() 
> methods.
>   Mentions 'two paths are equal' but I don't see the connection.
>
> The @Override's are fairly clear all by themselves, but the 
> @SuppressWarnings probably deserve
> a short comment as to why the warning is being suppressed rather than 
> addressed in the code.
> Even if it is a short comment like "// Serialization not used"
>
> On imports, I like that you have deleted many unneeded ones. I myself 
> prefer to completely
> avoid wildcard imports, using an IDE to keep them accurate, but that's 
> just my style.
>
> I'm a little concerned that we might be disturbing the dust and 
> cobwebs on these demos, I'm
> assuming that all the demo tests will be run, and the jdb tests, but 
> overall I'm glad they are getting
> some attention.
>
> -kto
>
> On Apr 18, 2011, at 5:01 AM, Nils Loodin wrote:
>
>> Hello all!
>> My name is Nils Loodin ,and I work in the swedish SQE team, now under
>> Ken chen. I've been working on the refresh demos project (PRD req. no.
>> 537450)
>>
>> I actually sent a request for a review previously, but that mail seems to have gone missing (thanks Beehive), but it's available at the mailing list archive:
>> (http://mail.openjdk.java.net/pipermail/serviceability-dev/2011-April/003868.html)
>>
>> Anyway, that might be for the better, since I've had time to update the webrev with some more stuff after comments from the nice Mandy Chung!
>>
>> One thing to note though, is that this was a pretty time bound project, but with a lot of code to go through and update. There are definitely things to do, still. perhaps this can be reviewed as an initial effort.
>>
>> (Overview here:
>> https://sunspace.sfbay.sun.com/display/JAVASE/JDK+7+Demos+Update)
>>
>> The changes should be pretty trivial. Mostly added annotations, using
>> for-each constructs and generics instead of raw types.
>>
>> Please have a look here:
>> http://wikifiles.se.oracle.com/qa/nloodin/7029383/webrev.01/  <http://wikifiles.se.oracle.com/qa/nloodin/7029383/webrev.00/>
>>
>> Regards
>> Nils Loodin
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20110421/a04352ea/attachment-0001.html 


More information about the serviceability-dev mailing list