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.

Bug 126741 - Java Project Common API module
Summary: Java Project Common API module
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Project (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Tomas Mysik
URL:
Keywords: API, API_REVIEW_FAST
: 63679 126740 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-02-05 16:31 UTC by Tomas Mysik
Modified: 2008-02-22 13:04 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
proposed patch (828.78 KB, patch)
2008-02-05 16:37 UTC, Tomas Mysik
Details | Diff
javadoc (121.69 KB, application/x-compressed)
2008-02-05 16:39 UTC, Tomas Mysik
Details
hg bundle (attaching after discussion with Tomas Zezula) (165.29 KB, application/octet-stream)
2008-02-07 12:08 UTC, Tomas Mysik
Details
Diff stat of patch (21.13 KB, text/plain)
2008-02-07 17:50 UTC, Jesse Glick
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Mysik 2008-02-05 16:31:23 UTC
Java Project Common API module should contain code which is more or less copy-pasted from J2SE project type to Java EE 
project types (Web, EJB, AppClient project types). This module should provide friend API for all the project types.
The first phase includes SourceRoots, UpdateHelper and different *Query implementations, e.g. FileEncodingQuery. The 
next phase is focused on class path, but now it's still work in progress (because of shared libraries).
I will attach javadoc and patch.

I'm also going to push code related to class path but these packages are not public yet and of course another API 
review will be done for it after the changes are finalized.
I can provide the whole hg bundle if needed.
Comment 1 Tomas Mysik 2008-02-05 16:37:47 UTC
Created attachment 56088 [details]
proposed patch
Comment 2 Tomas Mysik 2008-02-05 16:39:11 UTC
Created attachment 56089 [details]
javadoc
Comment 3 Tomas Mysik 2008-02-07 12:08:53 UTC
Created attachment 56230 [details]
hg bundle (attaching after discussion with Tomas Zezula)
Comment 4 Jesse Glick 2008-02-07 17:49:34 UTC
[JG01] Not sure if you care, but: UpdateHelper/Implementation do not look like they could be used to implement automatic
selection of project.xml schema version according to XML validation the way ant.freeform does. For that to work you
would need to have the Element of the proposed new XML content available when deciding which version to write out.

[JG02] Some classes, for example ClassPathItem, have no Javadoc.

[JG03] Delete the empty XML layer.

BTW maybe I'm missing something, but diffstat (will attach) does not report much actual reduction in the total amount of
code as a result of this patch. Might just be incorrect reporting.
Comment 5 Jesse Glick 2008-02-07 17:50:13 UTC
Created attachment 56270 [details]
Diff stat of patch
Comment 6 Tomas Mysik 2008-02-08 10:29:00 UTC
[JG01] In fact, you are right. In this step I just wanted to remove duplicity and create only one UpdateHelper. 
Moreover UpdateHelper should be integrated into AntProjectHelper and so there will be more work on it (see issue 
#122655).

[JG02] Yes, I know that some classes don't have Javadoc - but all these classes should be in non-public packages. 
These classes are work in progress and will be probably more or less changed because of shared libraries. That's the 
main reason why I cannot remove duplicates in project's class path files.

[JG03] Empty XML layer removed. Should I upload new hg bundle? Just this one change made.

To diffstat: your diffstat belongs to the whole hg bundle - without class path related code the number is about 4600 
lines "better". The reason why pushing this code as well is explained in the initial post.

Thank you Jesse for your comments.
Comment 7 Jesse Glick 2008-02-08 16:09:21 UTC
No need for a new bundle just because of JG03.
Comment 8 Tomas Mysik 2008-02-11 08:00:03 UTC
If there are no objections, I'll integrate tomorrow. Thanks.
Comment 9 Tomas Zezula 2008-02-11 09:54:44 UTC
I have no objections, all of them was solved during the design of the API.
Please, integrate it.
Comment 10 Jaroslav Tulach 2008-02-11 22:28:02 UTC
It is refreshing to see so much removed code while the functionality removes the same.

Y01 Imho SourceRoots shall not be an interface, but a final class (at least I do not see anyone directly implementing 
it). Making it a class will allow future extensions of its methods and potentially behaviour...
Comment 11 Tomas Mysik 2008-02-12 06:52:35 UTC
Y01 Yes, probably good idea - I will do it, not big change.

Thanks for review.
Comment 12 Tomas Mysik 2008-02-12 11:49:15 UTC
Pushed to main.
Comment 13 Jesse Glick 2008-02-20 23:38:50 UTC
By the way you neglected to include this issue number in any of the commit log messages that I could see, making it hard
to find this issue.

You might have missed some impls - at least EarSources.java looks like it could use this API.
Comment 14 Jesse Glick 2008-02-20 23:39:00 UTC
*** Issue 63679 has been marked as a duplicate of this issue. ***
Comment 15 Tomas Mysik 2008-02-21 07:49:07 UTC
> By the way you neglected to include this issue number in any of the commit log messages that I could see, making it 
> hard to find this issue.

Yes, that's true but - in which commit log should the issue number appear? It was quite many commits so - in the last 
one? In the merge commit? I was thinking about it but didn't see any correct solution - what's the right way how to do 
it? Thanks for any suggestion.

> You might have missed some impls - at least EarSources.java looks like it could use this API.

Yes, could be. There are many nearly copy pasted classes in our project types... :/ In fact I focused mainly on Web, 
EJB and AppClient project types because EAR project is quite a lot different. Thanks for catching it Jesse, I will 
look at EAR project soon.
Comment 16 Jesse Glick 2008-02-21 16:02:22 UTC
You could I guess mention the issue # in the first commit adding the basic source files.

There is no apichanges.xml for the module, which would normally be the place to put an initial entry "API added..."
mentioning the issue. Since it's a friend API only, this is not so important though.
Comment 17 Tomas Mysik 2008-02-21 22:05:27 UTC
> You could I guess mention the issue # in the first commit adding the basic source files.

I will try to do it next time (the problem here is that this issue was filed when I was nearly finished).

> There is no apichanges.xml for the module...

I can add it if needed.
Comment 18 Tomas Mysik 2008-02-22 13:04:50 UTC
*** Issue 126740 has been marked as a duplicate of this issue. ***