Looks good to me. Cheers, Mike Swingler Apple Inc.
On May 5, 2012, at 5:57 AM, Anthony Petrov wrote: > Thanks for reviewing that. Your suggestion sounds reasonable, so I've > published a new webrev at: > > http://cr.openjdk.java.net/~anthony/8-26-windowListInDockMenu-7149062.2/ > > -- > best regards, > Anthony > > On 5/5/2012 1:50 AM, Mike Swingler wrote: >> Overall, this looks good, but my one suggestion would be to NOT break out >> AWTWindow_Normal and AWTWindow_Panel into their own files, but instead to >> put them side-by-side in the AWTWindow implementation. >> You could possibly share part of their implementations in a macro to enforce >> that they don't fall out of sync with each other over time, but that might >> be overkill - though I think keeping them in the same file will be important >> for people to see both at the same time. >> Regards, >> Mike Swingler >> Apple Inc. >> On May 4, 2012, at 9:09 AM, Anthony Petrov wrote: >>> Thanks for the review. >>> >>> -- >>> best regards, >>> Anthony >>> >>> On 5/4/2012 8:07 PM, Scott Kovatch wrote: >>>> On May 4, 2012, at 4:43 AM, Anthony Petrov wrote: >>>>> I've investigated that, but it looks like a category (or an extension) >>>>> requires a class name to add methods to. It's impossible to define an >>>>> anonymous category and then apply it to two different classes. However, >>>>> for the sake of this fix we do need to have two distinct classes: one >>>>> inheriting from NSWindow, and another - from NSPanel. I can't find >>>>> anything Obj-C-wise that could help eliminate the code replication here. >>>>> Well, we could extract it into a separate file and #include it within the >>>>> corresponding @implementation sections. But given the really small size >>>>> of the replicated code this would look a bit weird in my opinion. >>>> No problem -- it was more a random thought than a concrete suggestion. I >>>> doubt we will be changing much in either file over time. >>>>> So I re-generated the webrev w/o the UTILITY thing mentioned above. The >>>>> final webrev is at: >>>>> >>>>> http://cr.openjdk.java.net/~anthony/8-26-windowListInDockMenu-7149062.1/ >>>>> >>>>> Looks OK to push that? >>>> Looks good to me. >>>> -- Scott K.
