RFR: 8244297: Provide utility for testing for memory leaks [v6]
Florian Kirmaier
fkirmaier at openjdk.java.net
Thu Oct 22 20:03:11 UTC 2020
On Sat, 17 Oct 2020 14:04:54 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> This is looking quite good now. I really like how it simplifies the tests that you modified to use it. I'll run the tests next week after you update the PR.
>>
>>> I don't plan to confirm all coding guidelines of the JFX-project inside of JMemoryBuddy.
>>
>> Which ones in particular? I would think that the white space guidelines should be easy to adopt, although since this is a "downstream" implementation, we can relax this a bit. The only spacing issue I see in the code that really stands out to me is the missing space after conditional and loop operators (a favorite saying of mine is that "`if` is not a method"). I note that while different coding style guides vary on many points, all of the ones I have looked at recommend a space separating these operators from the opening paren. Do you think it would cause you problems to adopt this in the upstream code?
>>
>> I'm not really concerned about the naming of non-public variables: as long as the existing names aren't confusing (which I don't think they are), you can take our suggestions as just that -- suggestions that you can adopt or not.
>
> One more thing: I don't know if you noticed this from the Skara bot:
>
>> JDK-8244297: Provide utility for testing for memory leaks ⚠️ Title mismatch between PR and JBS.
>
> You will need to update the PR title to match the JBS issue title.
First, thank you both for all your comments!
I integrated most of the suggested changes into the class and I will commit them back to the original project when the PR is closed.
I've adopted the whitespace after if, and integrated most of the suggested changes (but not all)!
The title should be correct now.
-------------
PR: https://git.openjdk.java.net/jfx/pull/204
More information about the openjfx-dev
mailing list