On Tue, 5 Nov 2024 22:31:14 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Kevin Rushforth has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains six additional >> commits since the last revision: >> >> - Add pointer to CSR in JBS >> - Remove extra blank lines >> - Merge branch 'master' into 8309381-incubator.dev >> - Merge branch 'master' into 8309381-incubator.dev >> - Remove call to doPrivileged >> - 8309381: Support JavaFX incubator modules > > build.gradle line 2877: > >> 2875: // Add a project declaration for each incubator module here, leaving >> the >> 2876: // incubator placeholder lines as an example. >> 2877: // BEGIN: incubator placeholder > > Q: maybe we should mention the JBS every time we say the word "incubator"? > > It might be useful to anyone who is looking at the code and has no access to > git history (or when git history is obscured by a move). I mean, JBS is our > knowledge base, and it usually helps. I generally prefer not to do this. In this case, the JBS Enhancement isn't what we would want to point to anyway (it would just be a placeholder). I can see some value in pointing to the CSR, so I'll add that here (I don't think there a need to repeat it). What would really be useful, though, is a pointer to a template with a sample module, along the lines of what I mentioned in #1617, and a pointer to the JEP (which gets back to an earlier discussion of a permanent repository for JEPs). I'll file a follow-up issue to address this. > settings.gradle line 42: > >> 40: "web", >> 41: "media", >> 42: "systemTests" > > thank you for placing each entry on a separate line! > > I would prefer to remove the blank lines around the comment so as not to > break the visual grouping (the comments are typically syntax colored anyway > unless the user is on TRS80) OK ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1616#discussion_r1866245340 PR Review Comment: https://git.openjdk.org/jfx/pull/1616#discussion_r1866249974
