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: | API review of SourceForBinaryQuery extension | ||
---|---|---|---|
Product: | java | Reporter: | Tomas Zezula <tzezula> |
Component: | Classpath | Assignee: | Tomas Zezula <tzezula> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | dkonecny, jglick, jlahoda, mkleint, phejl |
Priority: | P2 | Keywords: | API, API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | TASK | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 128353, 198951 | ||
Attachments: |
Javadoc
Patch Diff Javadoc |
Description
Tomas Zezula
2008-02-28 13:35:41 UTC
Created attachment 57448 [details]
Javadoc
Created attachment 57449 [details]
Patch
[JG01] <author login="jglick"/>? <issue number="128353"/>? <date day="28" month="2" year="2008"/> when it cannot anyway be committed for a wekk? Please check over apichanges.xml. (BTW you may be interested in: http://wiki.netbeans.org/HgHowTos#section-HgHowTos-DevelopAPIReviewPatchesUsingMQ) [JG02] Why the calls to l.getClass() in add/removeCL? [JG03] Use ChangeSupport. [JG04] 'public' and 'static' modifiers inside an interface are redundant and can be deleted. [JG05] SFBQI2.findSourceRoots2 Javadoc should be made very brief and link to SFBQI.fSR for all details to ease maintenance. JG01: Fixed JG02: Prevents null as a parameter, immediately throws NPE, nicer than if (a == null) throw new NPE(); JG03: ChangeSupport can be used it does the same. It has an unit test, so I will use it, it's new for me. Thanks or pointing me on it. JG04: I know that these modifiers are implicit inside interface, but I prefer to list them anyway - better readability from my point of view. JG05: Putting @see instead of javadoc should work fine, but the code completion will show just a link. I don't have strong opinion on this. [JG02] - use o.o.u.Parameters [JG05] - sure, but you can follow this link in CC by clicking on it. I am just saying that I see a lot of things I would want to edit in the current Javadoc, and I would not want to have to edit it in two places. the api change looks ok to me. mk1: the getSources() in the Result2 in api package seems to have no javadoc mk2: the implementation part has the javadoc but it's not immediately clear what preferring binary or source actually means. mk3: I haven't checked the actual code but I assume if some result is the old non-Result2 one, the old heuristics is still performed? My specific usecase is maven support which provides only sources in jars for binary roots without a project. Will that keep on working or I need to implement the Result2 immediately.. mk1: Already fixed. mk2: It contains brief info how to implement it, the complete description of preferSources is in SFBQI2 Javadoc which is a top level of this method, but I can add a see. Copying the same javadoc into it is probably not a good solution. mk3: Yes,no. (The heuristic is performed in the Result2 API class for original SPI Result, no heuristic for SPI Result2). You will not need to implement anything, as stated in apichanges.xml - binary,source,semantically compatible. Created attachment 57985 [details]
Diff
Created attachment 57986 [details]
Javadoc
I've attached the final diff and javadoc. I am going to integrate the patch on Monday (3/10/08). Fixed in: bde895cd674e |