afs commented on PR #2420:
URL: https://github.com/apache/jena/pull/2420#issuecomment-2081626363

   All looks good for moving forward with this. There are some process steps 
necessary; the code, apart from license headers and Eclipse issues noted below, 
looks good.
   
   **1. Licenses**
   
   All the java files and the POM file need the Apache license header for 
contributed code. The text is not the same as a 3rd party use of the Apache 
license.
   
   The attached bash script 
[`fixlicense`](https://github.com/apache/jena/files/15143404/fixlicense.txt) 
(instructions in the file) will fixup the java. (`.txt` added to make it 
accepted to GH upload). The POM file needs the same comment at the top as 
`jena-arq/pom.xml`.
   
   It's important that these are done before the code is added to the Jena 
codebase. The project can't make changes in order to accept a contribution.
   
   **2. build**
   
   At the top level,
   
   ```
       mvn clean install
   ```
   
   should run. After fixing the license issues above, and after rebasing to 
current main, it does for me.
   
   **3. Code (compile)**
   
   While it builds using maven, but fails in Eclipse IDE. It looks like the 
eclipse compiler does not handle generics in the same way as OpenJDK. The 
attached file 
[pr2420-eclipse.txt](https://github.com/apache/jena/files/15143468/pr2420-eclipse.txt)
 contains a diff for building in Eclipse and with maven.
   
   **4. Javadoc**
   
   There are javadoc warnings. Some relate to IntelliJ as they look like 
specific items but are quite a few incorrect javadoc.
   
   Under Eclipse: 293 javadoc errors.
   
   With maven: 45 javadoc errors including about `@inheritDoc` and 
`com.github.sszuev.jena.ontapi.vocabulary`.
   
   When working with the whole codebase, for example, during a release, it's 
very useful to have as clean a build as practical so it is easy to see new 
warnings appearing.
   
   **5. ICLA** (Individual Contributors License Agreement)
   
   This is a significant contribution. we need an ICLA on-file.
   
   The form and instructions are available at
   
   https://www.apache.org/licenses/contributor-agreements.html#clas
   
   https://www.apache.org/licenses/icla.pdf
   
   Having read the [intellectual property clearance 
process](https://incubator.apache.org/ip-clearance/index.html) and looked at 
the list of previous contributions in the Foundation, I don't now think we need 
a Software Grant. We would if there was an owning organsiation involved, but as 
@ssvuev would be signing both, nothing is gained.
   
   Taking into account the code origin discussion here on the PR, please remove 
the mention "The original code" sentence in README.md. If you want to credit 
the inspiration or ideas, please feel free to.
   
   **6. Prepare for merge**
   
   It would be useful if you could rebase the PR to current `main`.
   
   For incorporating into the Jena codebase, we don't need all the development 
commits.
   
   My preference is that the PR in squashed to a single commit, with mesage 
like "GH-2160: ...".  I didn't see any groups of related commits that would be 
useful to keep together. It's better to do the suqash on the PR, not via github 
"squash and merge" so that it is merged into Jena exactly as contributed.
   
   **7. PMC VOTE**
   
   When it's ready, we'll have a formal VOTE on dev@jena to show the PMC is 
aware and agrees to what is going on.
   
   **8. Documentation - jena-site**
   
   At some point, not necessarily before merging the code, the Jena website 
will need at least one page describing the new ontapi.  If nothing else, 
something the release can point to and about how user can migratre code to 
using the new module. How much of the existing documentation still applies 
(except for the java package name).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to