Created attachment 109820 [details]
proposed patch

includes WebStart module update taking use of the new SPI

Y01 I don't see list of possible (useful) values returned from new getCategory() method enumerated anywhere. These strings will be part of exported API from the J2SE project module.

Y02 If the answer to Y01 is "only 'Run' is supported right now", then why can't you stick with the existing J2SERunConfigProvider interface and need to create new and deprecate the old?

Y03 I see no sign of a test.

Re Y01/Y02

Because of JavaFX 2.0 support we will need to extend the panels of at least two categories: Run and Build/Packaging. There is a real possibility that more will be needed in not too distant future due to ever expanding requirements from the JFX side. So the idea was to provide just one universal interface instead of several category-dependent ones.

getCategory() return values: I was not sure if there was a need to provide a fixed enumeration as different project extensions may need to extend different categories. It is true though, that to make this mechanism work it is necessary to adjust each affected J2SE panel for which this should be defined. (The adjustment is transparent). Some panels clearly can not be expanded due to the extent of their current contents, for others the extension could be senseless. But at least with respect to JFX it seems future-safe to allow extension for:

Build
Build/Packaging (this is needed now)
Build/Documenting
Run (this is needed now)
Application

If we settle on such list than we could have, e.g.,

public enum ExtendableCategory { BUILD, PACKAGING, DOCUMENTING, RUN, APPLICATION }

and 

ExtendableCategory getCategory();

and all the affected panels would be adjusted to support this.

TZ01: I am not a friend of extending all panels to support it. Enum can be used (as it's open to extensions). So if you prefer the enum you can add into it what's needed now (Build, Run) and add others when needed.

TZ02: What happens when more providers provide panel for same category?

Re TZ01: OK good point.

Re TZ02: All get included in extPanel(), positioned vertically under each other, starting from top.

OK, thanks.

Created attachment 109899 [details]
updated patch proposal

getCategory() return values now restricted to RUN and PACKAGING only.
patch includes adjustments to Run and Packaging panels,
update of WebStart module, and a test (comment #2 Y03)

[JG01] Minor point: "extendable" is not a word; you mean "extensible". Also in Javadoc I think by "unanimously" you meant "unambiguously".


[JG02] This is a friend API, so you can consider just deleting the obsolete SPI; to do so you should also increment the major release version on java.j2seproject to indicate the incompatibility, and push new versions of all current friends (webstart, hudsonfindbugs, swingapp) by incrementing their "spec" versions while making them request the /2 version. Retaining compatibility was good practice for public APIs :-) but deleting the dead code paths can make the remaining code easier to follow.

By the way, do not forget to update <date> in apichanges.xml before committing to trunk. (My habit is to preset it for a week from the initiation of the API review.)

Thanks, Jesse. Good comments..

.. only as for [JG02] I am not sure whether increasing major j2seproject version would not have possibly farther reaching consequences than the proposed SPIs themselves. I'm better asking Jarda Tulach for opinion...

(In reply to comment #10)
> I am not sure whether increasing major j2seproject
> version would not have possibly farther reaching consequences than the proposed
> SPIs themselves.

http://deadlock.netbeans.org/hudson/job/nbms-and-javadoc/lastSuccessfulBuild/artifact/nbbuild/build/generated/deps.txt is the place to check for all "known" dependencies on this module (i.e. main + contrib). Other modules "out there" could have a dependency on it if they

1. Do not expect to compile against its APIs, as e.g. java.examples. It is not always clear what the meaning of such a runtime-only dep is - the depending module is somehow expressing that it expresses the Java SE project type to be available.

2. Use an implementation dep, as e.g. projectimport.eclipse.j2se. Note that java.j2seproject currently specifies an integral OpenIDE-Module-Implementation-Version, meaning it has a kind of second pseudo-API; this number ought to be changed frequently. (We want to remove all such impl deps and explicit impl versions, but there has not been enough effort put into it yet.)

(In reply to comment #8)
> [JG02] This is a friend API, so you can consider just deleting the obsolete
> SPI; 

I am not sure it worths the problems. We have seen one example[1] of where an incompatible change in a friend API can lead during 7.0.1 release. I know 7.1 is not an update release, so yes, you can do an incompatible change, but I would not do it.

Anyway I have a feeling that JG02 is more an advice (TCA) than requirement (TCR). Jesse can correct me.

[1] http://wiki.apidesign.org/wiki/Incremental_deployment

(In reply to comment #12)
> I have a feeling that JG02 is more an advice (TCA) than requirement (TCR).

Yes, sorry if that was not clear.

In view of Comment #12 which demonstrates what I was afraid of I opt for backward compatibility and not changing the major j2seproject version at this moment, keeping the old SPI in existence as deprecated.

I plan to close this issue (if no other objections appear) on We 17 Apr morning so that we can proceed with JFX2 implementation.

pushed to jet-main

http://hg.netbeans.org/jet-main/rev/e4506e0cac4d

http://hg.netbeans.org/jet-main/rev/d7551eec65ea

http://hg.netbeans.org/jet-main/rev/b802593be412

Integrated into 'main-golden', will be available in build *201108180601* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/e4506e0cac4d
User: Petr Somol <psomol@netbeans.org>
Log: #200704 SPI to enable extensions to various J2SEProject category panels in Project Properties