I would like to ask for the review of the following API change.
Some components which use the Database Explorer API usually need to display the
list of database connections in the UI, usually in a combo box. Therefore I am
proposing the introduction of a DatabaseConnectionUIHelper class containing a
static connect(JComboBox) method, which would display the database connections
as items of the passed combo box, followed by a separator item and an Add
Database Connection item. The latter item displays the New Connection dialog
when selected, allowing new database connections to be added.
Currently there are three components which will make use of the
DatabaseConnectionUIHelper class: the toolbar of the SQL editor, the Create Data
Source dialog and the Entity Classes/CMP Beans from Database wizard.
The target milestone is NetBeans 5.5. The change is compatible, so I request a
Created attachment 29292 [details]
Created attachment 29293 [details]
There is missing @since tag in declaration of the connect method.
Maybe the method should take ConnectionManager as argument. I would not say
this if the ConnectionManager had only static methods, but in fact it has
instances and thus it makes sense to select the one to work on. If you do not
accept this comment then please at least improve javadoc to mention that the
method operates on ConnectionManager.getDefault().
Re. name of the class - I do not like it much, but I agree that the method
should be in separate class and not (for example) in ConnectionManager. Maybe
"DatabaseUIs" would be my preferred choice. However there is a more important
aspect that probably Jesse should comment on - he likes to separate APIs into
"necessary" and "helper" ones - this UI method is clearly just a helper which
everyone could write by themselves, so I expect Jesse to suggest to follow
convention of projects and to create separate package - org.nb.api.db.support
and put all such "helper" classes there.
PS: Why Libor is author of all changes in the diff and Andrei is submitting
Re. missing @since tag: I thought that it is not necessary to add @since tags to
all methods of a new class and that it is enough to put a tag on the class
itself. Am I wrong?
Re. ConnectionManager parameter: I agree. Although ConnectionManager is a
singleton now, this could save a deprecated method if in the future I decide it
shouldn't be a singleton.
Re. class and package name: I don't like the name either, but couldn't come up
with anything better. DatabaseUIs and o.n.api.db.support seem too generic to me.
The package name suggests it could contain more database-related APIs than those
from the db module, but no other module than db would be able to add claseses to
it, since we don't like shared packages. Same for the class. Therefore I would
Re. other submitter than author: I should have written this class at Libor's
request, but since Libor had already created a similar one for data sources, I
asked him to write this one too. He didn't have time to ask for a review because
he was busy preparing his own review of the data sources API.
I will attach a new diff when a consensus is reached over the naming.
All agreed and sorry for the confusion with @since tag.
Created attachment 29324 [details]
Proposed change (updated)
Created attachment 29325 [details]
If there are no more comments, the change will be commited to the release55
branch next week. Thank you for the review.
Changes were commited into release55 branch.