RFR: JDK-8290011: IGV: Remove dead code and cleanup [v18]
Tobias Holenstein
tholenstein at openjdk.org
Tue Oct 4 14:05:28 UTC 2022
On Thu, 8 Sep 2022 12:43:47 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:
> This changeset goes beyond trivial cleanups (removing dead code, trailing whitespace, legacy functionality, etc.), and it would help if you could summarize (and motivate if necessary) the main changes in it.
>
> I found that switching among opened graphs from different groups does not update anymore the highlighted graphs in the Outline window, nor the content of the Bytecode and Control Flow windows. Maybe an effect of splitting #10164?
>
> A few more comments:
>
> * I would also prefer to leave the `toString()` methods in, for ease of debugging.
> * Why are some tests in `InputGraphTest.java` removed? Were they not run before?
> * I agree with enforcing alphabetic order of imports, but I would personally prefer to import explicitly all individual classes rather than using wildcards (matter of taste though, I do not think we have any style guidelines for tools like IGV).
> * Please update the copyright headers, at least for files with non-trivial changes.
Hi @robcasloz ,
I updated the PR to the current master to resolve merge conflicts. Further I re-added `toString()` and printing functions as well as all the removed tests (`InputGraphTest.java`).
Regarding wildcards in imports I decided to leave them in since they were already used in the code before. The book `Code Complete` suggests to use wildcards for imports, whereas the google style guide for java argues against wildcards. I could not find anything in our style-guide, but personally I have no strong opinion on this matter.
I think the copyright headers are updated for all non-trivial changes, or where is it missing?
The PR should now be ready to be reviewed again.
Thanks!
-------------
PR: https://git.openjdk.org/jdk/pull/10197
More information about the hotspot-compiler-dev
mailing list