Re: Use switch expressions in jdk.internal.net.http and java.net.http
Hi Kartik, I ran your patch through our test suite and it came back fine so I’ve created a JIRA issue to track your fix: https://bugs.openjdk.java.net/browse/JDK-8257401. Now that you have a bug ID for your changes, can you update the commit message for the PR to use the format `bug id : message` i.e. `8257401: Use switch expressions in jdk.internal.net.http and java.net.http` If you need any help with this please let me know. Kind regards, Patrick > On 25 Nov 2020, at 16:48, Patrick Concannon > wrote: > > Hi Kartik, > > Thanks for running the tests. I’m not sure about your problem on Ubuntu > 20.10, but I’ll take a look into it. > > Just so you know, you can also run tests using jtreg. You can find more > information on how to do that here: > http://openjdk.java.net/jtreg/runtests.html > > I’ll import your patch now, and run it against our test suites. All going > well, I can sponsor the PR and then hopefully we can integrate it into the > JDK for you! > > Kind regards, > Patrick > >> On 25 Nov 2020, at 11:28, Kartik Ohri wrote: >> >> Hi Chris and Patrick, >> It is the first time I am running the tier2 tests so I am not sure if I am >> doing it correctly. I'll share what I did and my observations. >> >> I executed make run-test-tier2 on my Ubuntu 20.10 machine and the tests >> failed to build due to some warnings in a hostpot jtreg test. The output was >> as follows: >> >> * For target >> support_test_hotspot_jtreg_native_support_exesigtest_exesigtest.o: >> /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c: >> In function 'setSignalHandler': >> /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c:245:9: >> error: 'sigset' is deprecated: Use the signal and sigprocmask functions >> instead [-Werror=deprecated-declarations] >> 245 | sigset(signal_num, handler); >> | ^~ >> In file included from >> /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c:25: >> /usr/include/signal.h:353:23: note: declared here >> 353 | extern __sighandler_t sigset (int __sig, __sighandler_t __disp) >> __THROW >> | ^~ >> cc1: all warnings being treated as errors >> >> >> However, when I ran the same command on my Ubuntu 20.04 machine, I got the >> following output. >> == >> Test summary >> == >> TEST TOTAL PASS FAIL ERROR >> >> jtreg:test/jdk:tier2 3664 3664 0 0 >> >> jtreg:test/langtools:tier2 1111 0 0 >> >> jtreg:test/jaxp:tier2 448 448 0 0 >> >> == >> TEST SUCCESS >> >> So, I think the tests passed on Ubuntu 20.04 but failed to execute on Ubuntu >> 20.10. >> I tried to find a reason for this and found >> https://sourceware.org/pipermail/libc-alpha/2020-May/113971.html. I think >> sigset got deprecated after Ubuntu 20.04 but so that is probably a different >> issue and not related to this patch. >> >> I do not have access to any other machine so I could not run the tests on >> Windows, macOS and so on. >> >> Thanks. >> >> Regards, >> Kartik. >> >> On Mon, Nov 23, 2020 at 11:37 PM Patrick Concannon >> wrote: >> Hi Kartik, >> >> Thanks for submitting the patch. Once you’ve run the tier2 tests, I’d be >> happy to sponsor it for you. >> >> -Patrick >> >>> On 23 Nov 2020, at 09:09, Chris Hegarty wrote: >>> >>> Hi Kartik, >>> On 21 Nov 2020, at 12:01, Kartik Ohri wrote: Hi! I would like to submit this patch https://github.com/openjdk/jdk/pull/1364 with the rationale to improve the readability of the code. Can someone please take a look at it and create a public issue if the patch is OK to be included ? >>> >>> This certainly seems like a reasonable thing to do. >>> >>> Can you please run tier2 testing, since it contains the tests for the HTTP >>> Client. >>> >>> Thanks, >>> -Chris. >> >
RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http
Hi! Kindly review this patch to replace switch statements with switch expressions (where it makes sense) in the http client modules. The rationale is to improve readability of the code. Regards, Kartik - Commit messages: - Use switch expressions in jdk.internal.net.http and java.net.http Changes: https://git.openjdk.java.net/jdk/pull/1364/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1364&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8257401 Stats: 270 lines in 17 files changed: 1 ins; 137 del; 132 mod Patch: https://git.openjdk.java.net/jdk/pull/1364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1364/head:pull/1364 PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v2]
> Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik Kartik Ohri has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http - Changes: - all: https://git.openjdk.java.net/jdk/pull/1364/files - new: https://git.openjdk.java.net/jdk/pull/1364/files/4ecf4e0d..542298e0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1364&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1364&range=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1364/head:pull/1364 PR: https://git.openjdk.java.net/jdk/pull/1364
Re: Use switch expressions in jdk.internal.net.http and java.net.http
Hi Patrick, Thank you for testing the patch and creating the issue. I have updated the commit message and the PR title as well. Regards, Kartik On Mon, Nov 30, 2020 at 6:14 PM Patrick Concannon < patrick.concan...@oracle.com> wrote: > Hi Kartik, > > I ran your patch through our test suite and it came back fine so I’ve > created a JIRA issue to track your fix: > https://bugs.openjdk.java.net/browse/JDK-8257401. > > Now that you have a bug ID for your changes, can you update the commit > message for the PR to use the format `bug id : message` i.e. `8257401: Use > switch expressions in jdk.internal.net.http and java.net.http` > > If you need any help with this please let me know. > > Kind regards, > Patrick > > > On 25 Nov 2020, at 16:48, Patrick Concannon < > patrick.concan...@oracle.com> wrote: > > > > Hi Kartik, > > > > Thanks for running the tests. I’m not sure about your problem on Ubuntu > 20.10, but I’ll take a look into it. > > > > Just so you know, you can also run tests using jtreg. You can find more > information on how to do that here: > http://openjdk.java.net/jtreg/runtests.html > > > > I’ll import your patch now, and run it against our test suites. All > going well, I can sponsor the PR and then hopefully we can integrate it > into the JDK for you! > > > > Kind regards, > > Patrick > > > >> On 25 Nov 2020, at 11:28, Kartik Ohri wrote: > >> > >> Hi Chris and Patrick, > >> It is the first time I am running the tier2 tests so I am not sure if I > am doing it correctly. I'll share what I did and my observations. > >> > >> I executed make run-test-tier2 on my Ubuntu 20.10 machine and the tests > failed to build due to some warnings in a hostpot jtreg test. The output > was as follows: > >> > >> * For target > support_test_hotspot_jtreg_native_support_exesigtest_exesigtest.o: > >> > /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c: > In function 'setSignalHandler': > >> > /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c:245:9: > error: 'sigset' is deprecated: Use the signal and sigprocmask functions > instead [-Werror=deprecated-declarations] > >> 245 | sigset(signal_num, handler); > >> | ^~ > >> In file included from > /home/lucifer/IdeaProjects/jdk/test/hotspot/jtreg/runtime/signal/exesigtest.c:25: > >> /usr/include/signal.h:353:23: note: declared here > >> 353 | extern __sighandler_t sigset (int __sig, __sighandler_t __disp) > __THROW > >> | ^~ > >> cc1: all warnings being treated as errors > >> > >> > >> However, when I ran the same command on my Ubuntu 20.04 machine, I got > the following output. > >> == > >> Test summary > >> == > >> TEST TOTAL PASS FAIL > ERROR > >> jtreg:test/jdk:tier2 3664 3664 0 >0 > >> jtreg:test/langtools:tier2 1111 0 >0 > >> jtreg:test/jaxp:tier2 448 448 0 >0 > >> == > >> TEST SUCCESS > >> > >> So, I think the tests passed on Ubuntu 20.04 but failed to execute on > Ubuntu 20.10. > >> I tried to find a reason for this and found > https://sourceware.org/pipermail/libc-alpha/2020-May/113971.html. I think > sigset got deprecated after Ubuntu 20.04 but so that is probably a > different issue and not related to this patch. > >> > >> I do not have access to any other machine so I could not run the tests > on Windows, macOS and so on. > >> > >> Thanks. > >> > >> Regards, > >> Kartik. > >> > >> On Mon, Nov 23, 2020 at 11:37 PM Patrick Concannon < > patrick.concan...@oracle.com> wrote: > >> Hi Kartik, > >> > >> Thanks for submitting the patch. Once you’ve run the tier2 tests, I’d > be happy to sponsor it for you. > >> > >> -Patrick > >> > >>> On 23 Nov 2020, at 09:09, Chris Hegarty > wrote: > >>> > >>> Hi Kartik, > >>> > On 21 Nov 2020, at 12:01, Kartik Ohri wrote: > > Hi! > I would like to submit this patch > https://github.com/openjdk/jdk/pull/1364 with the rationale to improve > the readability of the code. Can someone please take a look at it and > create a public issue if the patch is OK to be included ? > >>> > >>> This certainly seems like a reasonable thing to do. > >>> > >>> Can you please run tier2 testing, since it contains the tests for the > HTTP Client. > >>> > >>> Thanks, > >>> -Chris. > >> > > > >
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http
On Sat, 21 Nov 2020 11:45:42 GMT, Kartik Ohri wrote: > Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik Just a question. Do we have clear guideline or requirement for using this new lang feature? We know there are a lot of old switch-case blocks in the existing codes, including source codes and test codes. - PR: https://git.openjdk.java.net/jdk/pull/1364
