On Sat, 17 Oct 2020 14:02:39 GMT, Kevin Rushforth <k...@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
