-
Notifications
You must be signed in to change notification settings - Fork 835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NETBEANS-1974] Gradle JavaEE Support #1215
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found a single test. That would be helpful. I left few comments inline. Nothing looks like a blocker.
} | ||
|
||
@Override | ||
public void beforeSave(WindowSystemEvent event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I do not really know why this one implemented this way, though it is how the Maven and Ant integration of the web project does that. I've just repeated the pattern.
} | ||
|
||
private String showBrowserOnRun() { | ||
Boolean show = (Boolean) project.getProjectDirectory().getAttribute(CustomizerRunWar.PROP_SHOW_IN_BROWSER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attribute on a filesystem? In case of projects I'd expect some AuxiliaryConfiguration
to store it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how Maven Java EE does it. Though I do not have to follow that. I'm going to change the implementation.
<!DOCTYPE filesystem PUBLIC "-//NetBeans//DTD Filesystem 1.2//EN" "http://www.netbeans.org/dtds/filesystem-1_2.dtd"> | ||
<filesystem> | ||
<folder name="Projects"> | ||
<folder name="com-kenai-lkishalmi-netbeans-gradle"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-Apache package name is surprising.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, to point that out. Leftover from the refactoring.
@@ -228,6 +228,7 @@ | |||
<friend-packages> | |||
<friend>com.sun.eview.project</friend> | |||
<friend>org.codehaus.mevenide.netbeans.j2ee</friend> | |||
<friend>org.netbeans.modules.gradle.javaee</friend> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to open to up these modules to general use and just state that they are "under development".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be certainly welcome, though that may be a different issue and a different PR.
server is added from the wizard itself.
Let's have this on the master. I hope Java EE 8 arrives there soon, so I can integrate that one and the debug support. |
No description provided.