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 202516 - APIReview: adding TemplateWizardFactory into Java Source Queries
Summary: APIReview: adding TemplateWizardFactory into Java Source Queries
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Source (show other bugs)
Version: 7.1
Hardware: All All
: P3 normal (vote)
Assignee: apireviews
URL:
Keywords: API, API_REVIEW_FAST
Depends on:
Blocks:
 
Reported: 2011-09-22 15:48 UTC by Jan Horvath
Modified: 2011-09-28 14:20 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Diff file (18.76 KB, patch)
2011-09-22 15:51 UTC, Jan Horvath
Details | Diff
Second part of the diff (9.37 KB, patch)
2011-09-23 10:06 UTC, Jan Horvath
Details | Diff
fixed diff (57.04 KB, patch)
2011-09-26 12:25 UTC, Jan Horvath
Details | Diff
Diff showing rename (31.53 KB, patch)
2011-09-26 20:31 UTC, Jesse Glick
Details | Diff
diff (30.49 KB, patch)
2011-09-27 14:45 UTC, Jan Horvath
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Horvath 2011-09-22 15:48:41 UTC
To allow template wizards to have the single entry point in JDev and NetBeans, I have implemented TemplateWizardFactory, which finds appropriate implementation of TemplateWizardProvider depending on the IDE.
Comment 1 Jan Horvath 2011-09-22 15:51:23 UTC
Created attachment 111055 [details]
Diff file
Comment 2 Jan Horvath 2011-09-23 10:06:37 UTC
Created attachment 111081 [details]
Second part of the diff
Comment 3 Tomas Zezula 2011-09-23 10:42:36 UTC
TZ01: The first diff does not contain any API change.
TZ02: The API in the second diff logically does not belong to Java Queries API.
TZ03: No Javadoc, No api changes, 
TZ04: Inc spec version
TZ05: TemplateWizardFactory has public constructor
TZ06: The provider = Lookup.getDefault().lookup(TemplateWizardProvider.class) in static initializer may cause problems to FoD.
TZ07: TemplateWizardFactory.create and TemplateWizardFactory.createForSuper uses raw types the same holds for TemplateWizardProvider.
Comment 4 Jan Horvath 2011-09-26 12:11:27 UTC
TZ01: fixed
TZ02: We don't have any other suitable module
TZ03: fixed
TZ04: fixed
TZ05: fixed
TZ06: fixed
TZ07: fixed

I plan to integrate the changes tomorrow before the public holiday.
Comment 5 Jan Horvath 2011-09-26 12:25:31 UTC
Created attachment 111183 [details]
fixed diff
Comment 6 Jesse Glick 2011-09-26 20:30:17 UTC
(Not really necessary to file an API review for a friend API like this, but fine if you are soliciting comments, and anyway good practice for public APIs.)


[JG01] Delete 'static TemplateWizardProvider provider' and just look up the instance whenever needed.


[JG02] TemplateWizardProvider has no class Javadoc, and very minimal method Javadoc.


[JG03] Javadoc tip: {@link WizardDescriptor.InstantiatingIterator} is just noisy when Javadoc will anyway be showing a link to the method's return type.


[JG04] Mercurial tip: be sure to use 'hg mv' (including Move refactoring in the IDE), not copy & delete. In this case

--- a/form.nb/src/org/netbeans/modules/nbform/wizard/TemplateWizardIterator.java
+++ /dev/null
@@ -1,343 +0,0 @@
......
--- /dev/null
+++ b/java.source.queriesimpl/src/org/netbeans/modules/java/source/queriesimpl/TemplateWizardIterator.java
@@ -0,0 +1,345 @@
....

gives no indication of what lines you actually changed when moving this class, beyond the obvious change of package declaration. I will attach a patch which fixes that, and which shows the addition of two apparently unused imports.
Comment 7 Jesse Glick 2011-09-26 20:31:22 UTC
Created attachment 111205 [details]
Diff showing rename

hg imp --no-commit /tmp/update.diff
hg addrem -s 90 form.nb java.source.queriesimpl
hg di > /tmp/update2.diff
Comment 8 Jan Horvath 2011-09-27 14:45:17 UTC
Created attachment 111242 [details]
diff

[JG01 - JG04] fixed
Comment 9 Jan Horvath 2011-09-27 15:20:36 UTC
b55d75aa00f9
Comment 10 Jesse Glick 2011-09-27 16:44:46 UTC
JG02 and JG03 were not in fact addressed, but they were trivial anyway.


By the way when mentioning a changeset it is good to give also the team repo you committed to, in case it has not yet propagated, e.g.:

jet-main #b55d75aa00f9

http://wiki.netbeans.org/BrowserTools#Link_to_Mercurial_Changesets can be used to hyperlink these.
Comment 11 Jaroslav Tulach 2011-09-28 14:20:37 UTC
Caused nbbuild test failure fixed in core-main#6175d93b4e71