RFR: 8273507: Convert test/jdk/java/nio/channels/Channels/TransferTo.java to TestNG test [v4]
Brian Burkhalter
bpb at openjdk.java.net
Mon Oct 18 17:03:49 UTC 2021
On Sat, 16 Oct 2021 13:28:03 GMT, Markus KARG <duke at openjdk.java.net> wrote:
>>> The comments are much improved but inconsistent. For example some of them have `@param` and other javadoc tags and some do not. There is also some variation in detail level.
>>
>> Is it essential for *this* PR that *all* comments must be consistent? I understood @AlanBateman in a way that it is enough to simply *have some* and honestly, I did not find such high quality comments in all the existing code you showed me. Regarding the detail level, it even makes sense to only go into details *where needed* IMHO to not repeat the obvious.
>>
>>> On a process level, it would be better to wait for feedback on changes before integrating. This is especially so as a sponsor has to be comfortable with the content. Ideally at least one approval should be associated with the most recent commit.
>>
>> Understood. I thought that it is irrelevant when I ask for integration, as I already covered all open issues and @shipilev already marked this PR as reviewed *as-is* (*without* any comments at all).
>
>> On a process level, it would be better to wait for feedback on changes before integrating. This is especially so as a sponsor has to be comfortable with the content. Ideally at least one approval should be associated with the most recent commit.
>
> @bplb I don't want to make it wrong again, but I think this time I am right that I can integrate *right now* (even I pushed more comments requested by you), as you approved this PR *directly after* you requested that comments. If not, please directly tell me how to otherwise understand your approval. Thanks.
@mkarg Yes you can go ahead and integrate.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5421
More information about the nio-dev
mailing list