[GitHub] matthiasblaesing commented on issue #877: [NETBEANS-1074] Module Review servletjspapi + web.jspparser

[GitBox <gi...@apache.org>]

matthiasblaesing commented on issue #877: [NETBEANS-1074] Module Review servletjspapi + web.jspparser
URL: https://github.com/apache/incubator-netbeans/pull/877#issuecomment-425637076
 
 
   @sarveshkesharwani we are heading into the right direction. For your questions:
   
   1. The license for the generated files follows the licenses used for the input sources. As a basic rule: If you don't have a license, you can't distribute it. In this case to be faithful to the input, I would declare that the combined files contain CDDL-1.0 and CDDL-1.1 licenses files -- this is true for the API, for the Parser I think I saw only CDDL-1.1.
   2. I think your observation is correct the target `create-jars-from-glassfish` is not executed automaticly. I would wager that this is an artifact of the times when maven central was not that popular/when the artifacts were not distributed on maven central. For the approach to regenerate the file every time the build is run: please don't. In `enterprise/web.core.syntax` you can see two thinks: the file is only generated when necessary (`-check-prepared-doc`) and only build when the release target is invoked (`<target name="-process.release.files" depends="prepare-doc"/>`). This can be adapted for these two modules. For `web.jspparser`, `ext-compile` looks like a good place to hook into and for the api the approach from `web.core.syntax` should be usable. I would remove the whole `create-jars-from-glassfish`  when you are done as I gets replaced by your work.
   3. Yes tests are pretty much broken. I assume when the donation was prepared `project3` and other files were not deemed donateable and thus removed from the codebase. You can recover at least parts of the unittest by commenting out line 108 in `org.netbeans.modules.web.jspparser.TestUtil#setup`. Then you would at least have a smoke test if something broke.
   
   Further comments:
   1. You'll need to revisit the construction of the `servlet3.1-jsp2.3-api.jar` file. You are packaging all schema files, while the original build only choose a subset (see `web.jspparser/build.xml` from line 138 onwards.
   2. Please follow the naming established in `web.core.syntax` for the generated artifacts: 1. prefix the filename with `generated-` and add a header to the license file `Type: generated`. Background: the `verify-all-libs-and-licenses` target of the netbeans build will flag the missing files for the licenses - but with the changes suggested in the answer 2. above the file will only be created when needed, so it would be a false positive. See for example `generated-jsf12-tlddoc-1.2-20-license.txt`  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists