This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Review of moving ProjectClassPathExtender into public package | ||
---|---|---|---|
Product: | java | Reporter: | Tomas Zezula <tzezula> |
Component: | Project | Assignee: | Tomas Zezula <tzezula> |
Status: | CLOSED FIXED | ||
Severity: | blocker | CC: | tpavek |
Priority: | P3 | Keywords: | API_REVIEW_FAST |
Version: | 4.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 49653 | ||
Attachments: |
Diff
Diff to previous patch: Added @since to ProjectClassPathExtender Diff to previous patch: Fixed arch.xml |
Description
Tomas Zezula
2005-01-05 15:25:49 UTC
Created attachment 19498 [details]
Diff
Changed interfaces: Added new public interface org.netbeans.spi.java.project.classpath.ProjectClassPathExtender Stability Category: Should become stable in the NetBeans 4.1 Created attachment 19525 [details]
Diff to previous patch: Added @since to ProjectClassPathExtender
Created attachment 19526 [details]
Diff to previous patch: Fixed arch.xml
I've reviewed the change. It looks ok, I agree with integrating it. The change seems fine to me as well. I agree with the integration. @since tag should use org.netbeans.modules.java.project/1, not just org.netbeans.modules.java.project. Re. j2seproject/build.xml change: I think some ide/* module now has an impl dep on j2seproject, alas, so this may not be possible any more. Note that your patch will no longer apply against trunk as is. Patch to j2seproject/.../project.xml is not related, is it? If not, please just do it in the trunk. Coordinate with dkonecny since he also has planned changes (in apireviews) to the AntArtifact interface, which I seem to recall would affect the addAntArtifact method signature. Now that the parameter types in the PCPE methods are all arrays, shouldn't the method names be pluralized? E.g. addLibraries, addArchiveFiles, addAntArtifacts. For that matter, what is the reason for this change? According to the Javadoc it seems that the semantics of calling pcpe.addLibrary(new Library[] {lib1, lib2}); are the same as pcpe.addLibrary(new Library[] {lib1}); pcpe.addLibrary(new Library[] {lib2}); so it seems an unnecessary complication to pass an array. In fact the only known client, form, still only adds one item at a time. Is there some hidden reason for this change? In apichanges.xml, you must use consistent capitalization: s/Classpath/classpath/ In apichanges.xml, please use <class> and <issue>. Don't forget Copyright 2005. s/can not/cannot/g; s/in the case when/in case/g IDE depends on java/j2seproject - I know, it is the new module. IDE does not depend on java/project - this dependency can be removed. The array is used due to performance, if the client needs to add more classpath entries it can use an array and project is saved just once. I will change the method names to plural. While taking look at David's scheduled changes I've decided to change the parameters from an array back to single object. It will be a bit slower for client of this API which needs to add more classpath entries, currently there is no one, but as the addArtifact(...) requires an additional argument, an URI of build output, the array version of the API will be ugly (addArtifacts (AntArtifact[] , URI[])). I've fixed all problems Jesse reported. Marking this issue as VERIFIED since the issue reporter == issue owner. |