RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java (was: Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java (was: Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java)

Ao Qi
On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman <[hidden email]> wrote:

>
> On 21/04/2020 04:58, Ao Qi wrote:
> >
> > On 2020/4/20 下午9:27, Alan Bateman wrote:
> >> On 20/04/2020 11:32, [hidden email] wrote:
> >>> Hi Alan,
> >>>
> >>> I don't remember it now. Likely a mistake. The changeset
> >>>
> >>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
> >>>
> >>> has that file. Perhaps it may be useful to restore & check if the
> >>> test passes.
> >> Yes. I think it was the only test for this jlink option so let's
> >> bring it back or replace it.
> >>
> >
> > I think the test should be brought back. I restored the test and it
> > didn't pass. Here is the diff against the original test[1]:
> > http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
> > is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
> > familiar with this field, so I am not sure I fix this rightly.
> Thanks, I think this looks okay.

Thanks, Alan. I have updated the bug title. Is another review needed?

Cheers,
Ao Qi

>
> -Alan.

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java (was: Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java)

sundararajan.athijegannathan
Looks good

-Sundar

On 27/04/20 12:24 pm, Ao Qi wrote:

> On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman <[hidden email]> wrote:
>> On 21/04/2020 04:58, Ao Qi wrote:
>>> On 2020/4/20 下午9:27, Alan Bateman wrote:
>>>> On 20/04/2020 11:32, [hidden email] wrote:
>>>>> Hi Alan,
>>>>>
>>>>> I don't remember it now. Likely a mistake. The changeset
>>>>>
>>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>>>>>
>>>>> has that file. Perhaps it may be useful to restore & check if the
>>>>> test passes.
>>>> Yes. I think it was the only test for this jlink option so let's
>>>> bring it back or replace it.
>>>>
>>> I think the test should be brought back. I restored the test and it
>>> didn't pass. Here is the diff against the original test[1]:
>>> http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
>>> is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
>>> familiar with this field, so I am not sure I fix this rightly.
>> Thanks, I think this looks okay.
> Thanks, Alan. I have updated the bug title. Is another review needed?
>
> Cheers,
> Ao Qi
>
>> -Alan.
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java (was: Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java)

Ao Qi
Thanks, Sundar!

I updated a new webrev (patch is the same, only hg commit info was
added): http://cr.openjdk.java.net/~aoqi/8242846/webrev.02/

Could someone help to sponsor this?

Thanks,
Ao Qi

On Mon, Apr 27, 2020 at 4:52 PM <[hidden email]> wrote:

>
> Looks good
>
> -Sundar
>
> On 27/04/20 12:24 pm, Ao Qi wrote:
> > On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman <[hidden email]> wrote:
> >> On 21/04/2020 04:58, Ao Qi wrote:
> >>> On 2020/4/20 下午9:27, Alan Bateman wrote:
> >>>> On 20/04/2020 11:32, [hidden email] wrote:
> >>>>> Hi Alan,
> >>>>>
> >>>>> I don't remember it now. Likely a mistake. The changeset
> >>>>>
> >>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
> >>>>>
> >>>>> has that file. Perhaps it may be useful to restore & check if the
> >>>>> test passes.
> >>>> Yes. I think it was the only test for this jlink option so let's
> >>>> bring it back or replace it.
> >>>>
> >>> I think the test should be brought back. I restored the test and it
> >>> didn't pass. Here is the diff against the original test[1]:
> >>> http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
> >>> is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
> >>> familiar with this field, so I am not sure I fix this rightly.
> >> Thanks, I think this looks okay.
> > Thanks, Alan. I have updated the bug title. Is another review needed?
> >
> > Cheers,
> > Ao Qi
> >
> >> -Alan.

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

sundararajan.athijegannathan
Hi,

Looks good. I'll sponsor this fix.

Thanks

-Sundar

On 27/04/20 4:15 pm, Ao Qi wrote:

> Thanks, Sundar!
>
> I updated a new webrev (patch is the same, only hg commit info was
> added): http://cr.openjdk.java.net/~aoqi/8242846/webrev.02/
>
> Could someone help to sponsor this?
>
> Thanks,
> Ao Qi
>
> On Mon, Apr 27, 2020 at 4:52 PM <[hidden email]> wrote:
>> Looks good
>>
>> -Sundar
>>
>> On 27/04/20 12:24 pm, Ao Qi wrote:
>>> On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman <[hidden email]> wrote:
>>>> On 21/04/2020 04:58, Ao Qi wrote:
>>>>> On 2020/4/20 下午9:27, Alan Bateman wrote:
>>>>>> On 20/04/2020 11:32, [hidden email] wrote:
>>>>>>> Hi Alan,
>>>>>>>
>>>>>>> I don't remember it now. Likely a mistake. The changeset
>>>>>>>
>>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>>>>>>>
>>>>>>> has that file. Perhaps it may be useful to restore & check if the
>>>>>>> test passes.
>>>>>> Yes. I think it was the only test for this jlink option so let's
>>>>>> bring it back or replace it.
>>>>>>
>>>>> I think the test should be brought back. I restored the test and it
>>>>> didn't pass. Here is the diff against the original test[1]:
>>>>> http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
>>>>> is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
>>>>> familiar with this field, so I am not sure I fix this rightly.
>>>> Thanks, I think this looks okay.
>>> Thanks, Alan. I have updated the bug title. Is another review needed?
>>>
>>> Cheers,
>>> Ao Qi
>>>
>>>> -Alan.
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Ao Qi
Thank you, Sundar.

On Mon, Apr 27, 2020 at 6:49 PM <[hidden email]> wrote:

>
> Hi,
>
> Looks good. I'll sponsor this fix.
>
> Thanks
>
> -Sundar
>
> On 27/04/20 4:15 pm, Ao Qi wrote:
> > Thanks, Sundar!
> >
> > I updated a new webrev (patch is the same, only hg commit info was
> > added): http://cr.openjdk.java.net/~aoqi/8242846/webrev.02/
> >
> > Could someone help to sponsor this?
> >
> > Thanks,
> > Ao Qi
> >
> > On Mon, Apr 27, 2020 at 4:52 PM <[hidden email]> wrote:
> >> Looks good
> >>
> >> -Sundar
> >>
> >> On 27/04/20 12:24 pm, Ao Qi wrote:
> >>> On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman <[hidden email]> wrote:
> >>>> On 21/04/2020 04:58, Ao Qi wrote:
> >>>>> On 2020/4/20 下午9:27, Alan Bateman wrote:
> >>>>>> On 20/04/2020 11:32, [hidden email] wrote:
> >>>>>>> Hi Alan,
> >>>>>>>
> >>>>>>> I don't remember it now. Likely a mistake. The changeset
> >>>>>>>
> >>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
> >>>>>>>
> >>>>>>> has that file. Perhaps it may be useful to restore & check if the
> >>>>>>> test passes.
> >>>>>> Yes. I think it was the only test for this jlink option so let's
> >>>>>> bring it back or replace it.
> >>>>>>
> >>>>> I think the test should be brought back. I restored the test and it
> >>>>> didn't pass. Here is the diff against the original test[1]:
> >>>>> http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
> >>>>> is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
> >>>>> familiar with this field, so I am not sure I fix this rightly.
> >>>> Thanks, I think this looks okay.
> >>> Thanks, Alan. I have updated the bug title. Is another review needed?
> >>>
> >>> Cheers,
> >>> Ao Qi
> >>>
> >>>> -Alan.