Hi Anuuraag, Aleksei,
Looks good to me.
Except that now copyright years must be updated to 2020.
No need for a new webrev if that's the only change!
best regards,
-- daniel
On 09/01/2020 17:17, Aleks Efimov wrote:
Hi Anuuraag,
Latest webrev: http://cr.openjdk.java.net/~aefimov/anuraaga/7006
Hi,
Could someone please review my fix for JDK-8237075 '@since tag missing
from DatagramSocket and MulticastSocket methods' ?
This trivial fix adds in the missing @since tags for methods listed
below. These methods were originally added in JDK 1.2.
java/net/DatagramSocket:
> public void co
On 14/01/2020 11:49, Patrick Concannon wrote:
Hi,
Could someone please review my fix for JDK-8237075 '@since tag missing
from DatagramSocket and MulticastSocket methods' ?
This trivial fix adds in the missing @since tags for methods listed
below. These methods were originally added in JDK 1.
On 14/01/2020 11:49, Patrick Concannon wrote:
bug: https://bugs.openjdk.java.net/browse/JDK-8237075
webrev: http://cr.openjdk.java.net/~pconcannon/8237075/webrevs/webrev.00/
Looks good to me as well.
best regards,
-- daniel
LGTM.
Surprising that we haven’t noticed this before, because the @since tags are
used, in part, to derive the symbol tables for --release ( maybe such old
version information is not useful these days )
-Chris.
> On 14 Jan 2020, at 11:49, Patrick Concannon
> wrote:
>
> Hi,
>
> Could someon
Daniel,
I imported the patch from the link you provided as follows:
hg import --no-commit open.patch
The patch applied successfully. I tried then to run the tests and saw that some
of them could not be compiled. For instance,
java/net/httpclient/websocket/BlowupOutputQueue.java
java
Hi Pavel,
That's strange. Are you sure your hg import worked properly?
Note that SecureSupport is a modified copy of Support
(not a rename) and DummySecureWebSocketServer is a
modified copy of DummyWebSocketServer (not a rename).
I wonder if this was causing issue with the import.
(the patch is
That patch contains the following lines:
--- old/test/jdk/java/net/httpclient/websocket/DummyWebSocketServer.java
2020-01-13 13:04:41.0 +
+++ /dev/null 2020-01-13 13:04:41.0 +
--- old/test/jdk/java/net/httpclient/websocket/Support.java 2020-01-13
13:04:44.00
On 14/01/2020 15:14, Daniel Fuchs wrote:
I wonder if this was causing issue with the import.
(the patch is obtained by hg diff, not hg export)
Yes. Damn. the open.patch generated by webrev renames the files
instead of copying/modifying...
I have added a proper changeset generated with `hg expo
Thanks Daniel!
Anuuraag,
I will update the copyrights, no need to send new patch. Then will
sponsor your change - commit and push to jdk/jdk.
Best Regards,
Aleksei
On 14/01/2020 11:06, Daniel Fuchs wrote:
Hi Anuuraag, Aleksei,
Looks good to me.
Except that now copyright years must be up
Hi Alan,
Globally this looks good to me. One small nit is that the `oldImpl`
field could now also become final (by applying the same trick you
did with createImpl - that is - have oldImpl() return a boolean
rather than set the field and assign the result in the constructor.
I've imported your pa
That changeset applies fine, thanks.
I was wondering what you had in mind when added OpeningHandshake:225.
Was it for general robustness or you ran into something in particular?
> On 14 Jan 2020, at 15:36, Daniel Fuchs wrote:
>
> On 14/01/2020 15:14, Daniel Fuchs wrote:
>> I wonder if this was
Hi Pavel,
On 14/01/2020 17:54, Pavel Rappo wrote:
That changeset applies fine, thanks.
I was wondering what you had in mind when added OpeningHandshake:225.
Was it for general robustness or you ran into something in particular?
More for general robustness. Sometimes assertion errors are fire
I think the WebSocket part of the changeset look good to me. I have gone through
the non-WebSocket part of the changes shallowly. I'm not an expert.
> On 14 Jan 2020, at 17:59, Daniel Fuchs wrote:
>
> Hi Pavel,
>
> On 14/01/2020 17:54, Pavel Rappo wrote:
>> That changeset applies fine, thanks.
On 14/01/2020 18:17, Pavel Rappo wrote:
I think the WebSocket part of the changeset look good to me. I have gone through
the non-WebSocket part of the changes shallowly. I'm not an expert.
Thanks!
Your review is appreciated!
I will wait until Michael or Chris review the rest of the changes.
b
On 14/01/2020 17:49, Daniel Fuchs wrote:
Hi Alan,
Globally this looks good to me. One small nit is that the `oldImpl`
field could now also become final (by applying the same trick you
did with createImpl - that is - have oldImpl() return a boolean
rather than set the field and assign the result
Looks good Alan!
best regards.
-- daniel
On 14/01/20 19:23, Alan Bateman wrote:
Thanks for going through this. I tried to keep the changes to
DatagramSocket to a minimum but I don't mind making an exception for
oldImpl. It's a slippery slope as there is a lot of technical debt in
this area.
Hi Aleksei, Daniel,
Great news, thanks a lot for all your help on cleaning up this patch!
Cheers,
- Anuraag
On Wed, Jan 15, 2020, 01:14 Aleks Efimov wrote:
> Thanks Daniel!
>
> Anuuraag,
> I will update the copyrights, no need to send new patch. Then will
> sponsor your change - commit and pu
18 matches
Mail list logo
