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