matthiasblaesing commented on issue #161: [NETBEANS-96] New PAC Script evaluation environment URL: https://github.com/apache/incubator-netbeans/pull/161#issuecomment-343670673 I gave it another look I have one further comment on the code itself - the spec version of lib.jna.platform needs to be incremented, as the change in the friend-packages indicates that the package needs an update. Some of the files are also missing license headers. I made these here: - based on a rebased+squashed changeset: https://github.com/matthiasblaesing/incubator-netbeans/tree/pr161-rebase https://github.com/matthiasblaesing/incubator-netbeans/commit/6e21bcdd1c5d9f3eab4868b5ed1245c94b9c02be - based on your branch: https://github.com/matthiasblaesing/incubator-netbeans/tree/pr161-review https://github.com/matthiasblaesing/incubator-netbeans/commit/5826080b5301846ff2e38cb2f004e0f470526cc4 One structural question: Is there a reason for the friend-exports? I don't see usage of the org.netbeans.core.network packages in org.netbeans.core. Is there a concrete use-case/usage you have planed? Is it really sensible to have a different implementation of the JS evaluation environment for the PAC, that is not better done in the core implementation? From my perspective that would be the last two things - I noticed this while I prepared the merge.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
