On Mon, 24 Oct 2022 20:05:51 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> Magnus Ihse Bursie has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix styleheet
>
> make/jdk/src/classes/build/tools/taglet/SealedGraph.java line 49:
>
>> 47: public final class SealedGraph implements Taglet {
>> 48: private static final String sealedGraphDotPath =
>> 49: System.getProperty("sealedGraphDotPath");
>
> The naming and use is slightly off here.
>
> 1. It names the _directory_ in which the graph `.dot` file will be placed.
> It's not a path for the `.dot` file itself.
> 2. it does double-duty as the equivalent of `enableModuleGraph` in the module
> graphs world.
Would `sealedDotOutputDir` be ok? I think 2. will kind of follow from the fact
that it is specified. Otherwise we could do like `enableSealedDotOutputInDir`
but I think that's getting too verbose. And having two different properties
just because of naming makes no sense, since then we need to start checking
that they are consistent.
> make/jdk/src/classes/build/tools/taglet/SealedGraph.java line 182:
>
>> 180: .append(lineSeparator())
>> 181: .append(" rankdir=\"BT\";")
>> 182: .append(lineSeparator());
>
> Note to future selves: if we come up with Yet Another Kind of Graph, we
> should consider creating a `DotBuilder` class to share/simplify the code.
There is a dot-build for the ModuleGraph, but it makes a lot of assumptions
about what kind of graph it is generating. I tried using that instead first,
but it led to too much cludgy code, or too wide refactorings.
-------------
PR: https://git.openjdk.org/jdk/pull/10761
