JDK-8259530: Generated docs contain MIT/GPL-licenced works without reproducing the licence #3954
Conversation
…producing the licence
|
|
|
@jonathan-gibbons The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
| try (OutputStream out = df.openOutputStream()) { | ||
| Files.copy(entry, out); | ||
| } |
There was a problem hiding this comment.
I'm surprised to see you using Files.copy(Path source, OutputStream out) instead of DocFile.copyFile(DocFile).
Last time I suggested we use Files.copy instead of the DocFile.copyFile in jdk.javadoc, you argued that the latter is more helpful to an end-user:
In terms of general philosophy, when any IO problem occurs, I think it is important to specify the file involved, to give the end-user the best information for that person to diagnose the external condition. To me, that is better and more specific than saying "error copying from A to B, something went wrong with one of them".
There was a problem hiding this comment.
Yeah, good call.
It required some tweaks to DocFile and DocFileFactory to make it happen, but it's a move in the right direction.
DocFileFactory used to be new and clever and encapsulated the different kinds of file system. Now, NIO is accepted and standard. I would still keep DocFile to encapsulate Path vs. FileObject, but there is potential cleanup to improve this part of the code, and (most notably) convert command-line options to use Path instead of String where that is appropriate. But that is a different PR.
| private StandardDocFile(Path file) { | ||
| StandardDocFile(Path file) { | ||
| this.file = file; | ||
| } | ||
|
|
||
| /** Create a StandardDocFile for a given location and relative path. */ | ||
| private StandardDocFile(Location location, DocPath path) { | ||
| StandardDocFile(Location location, DocPath path) { |
There was a problem hiding this comment.
Why did you remove these two private modifiers?
|
@jonathan-gibbons This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 215 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
|
|
/integrate |
openjdk
bot
added
integrated
|
@jonathan-gibbons Since your change was applied there have been 224 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit e9f3e32. |
Please review a change for JavaDoc, for the Standard Doclet to copy legal header files into the generated docs from a default or designated directory.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3954/head:pull/3954$ git checkout pull/3954Update a local copy of the PR:
$ git checkout pull/3954$ git pull https://git.openjdk.java.net/jdk pull/3954/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 3954View PR using the GUI difftool:
$ git pr show -t 3954Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3954.diff