Hi Mandy,
Mandy Chung <mandy.ch...@oracle.com> wrote on 22/03/2019 16:56:12: > From: Mandy Chung <mandy.ch...@oracle.com> > To: Joe Darcy <joe.da...@oracle.com>, Adam Farley8 <adam.far...@uk.ibm.com> > Cc: core-libs-dev <core-libs-dev@openjdk.java.net> > Date: 22/03/2019 16:58 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, > On 3/22/19 8:40 AM, Joe Darcy wrote: > > Please update distinct versions of a webrev (e.g. distinguished with > .1, .2 directory names) rather than overwriting a single one. This > make it easier for those coming to the review thread later to see > the evolution of the changes over time. > > +1 > > I had requested new test in the webrev during my review. That really > helps me, a reviewer, to keep track what has been changed easily. > It will also give you an idea how many revisions this review > involves when you started for a code review (as opposed to asking > for "how to fix this issue"). Ah, that makes sense. Thanks for explaining. :) > > I was asked to read the regression test that is attached to JBS issue [1] > I was asked to review a diff (cut-n-paste) on the mail when I > requested a webrev to include a regression test. [2] > > On Jan 31, 2019 [3], I includeed a link to the OpenJDK developer > guide and I was hoping you read the guideline and be familiar with > it which should help you contributing to the OpenJDK. I completely forgot about that. I appreciate the reminder, will review the guide now so I don't forget again. > > I was disappointed to get your conclusion: > Historically, the bigger the change I propose, the more months it takes > the OpenJDK community to approve. Perhaps I misspoke. The only changes I've been able to get into OpenJDK have taken months apiece, with the exception of typos and pure comment tweaks. My *hope* is that small change sets will reduce the amount of debate (in general, this is not aimed at you personally), and result in a faster turnaround. > > I had helped as much as I can to sponsor this patch even if you > refused what I requested to help my review easier. Refused? I don't recall refusing your help. I appreciate your help, and I believe I have done everything you have asked, because I can see that you are working hard to help me get this changeset in. The only times I have not done as asked is when I have questions first, to help me understand, or because I have legitimately forgotten. > > I expected that you ran JDK regression tests on local machine rather > than I caught it for you. Is that what you expected a reviewer to > do it for you? Won't you consider this a new revision to your > patch? You can do anything you like and tells a reviewer that I > should be smart enough to identify the diff in the previous patch. > I am not an educator and excuse me if my response is not what you > are looking for. > > Mandy Do I expect reviewers to run tests for me? No. I made a mistake by assuming this code change was so small that a test run beyond MethodHandlesGeneralTest was unnecessary. This was a learning experience, and you did the right thing by double-checking, and finding my mistake. Now I know better. As for a new revision to my patch, I think you are referring to the versioning mentioned above. Now you have kindly explained the value of this to me, I am happy to include the versioning for future changes. Past changes may be tricky, as I don't have them. Illustrates the benefits of versioning quite well, hehe. As for your "educator" comment, you don't need to worry. Your response was quite clear. Best Regards Adam Farley > > [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019- > January/058320.html > [2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019- > February/058544.html > [3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019- > January/058350.html > [4] https://mail.openjdk.java.net/pipermail/core-libs-dev/2019- > March/058784.html Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
