RFR: 8244297: memory leak test utility [v6]

Kevin Rushforth kcr at openjdk.java.net
Sat Oct 17 14:07:18 UTC 2020


On Sat, 17 Oct 2020 14:02:39 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8244297
>>   Fixed unit-test
>
> 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.

-------------

PR: https://git.openjdk.java.net/jfx/pull/204


More information about the openjfx-dev mailing list