[GitHub] incubator-netbeans issue #130: Cache external binaries downloaded from maven...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/130 > The proposal here is to cache them in the $HOME/.hgexternalbinaries, as other binaries are cached. +1 I actually assumed the existing code did that. ---
[GitHub] incubator-netbeans issue #52: [NETBEANS-54] Module review xml modules
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/52 I really believe this should be discussed on dev@. There are two email threads, one started by me and one by @junichi11 for mentors (which haven't replied). We are not running in circles, you just want license headers everywhere! There are various types of 'templates': 1. example text displayed to users as preview in Options window and elsewhere 2. freemarker templates used internally and 3. freemarker templates the user is also able to see and edit (eg. Open in Editor for Tools | Templates). The general direction I am using is simple: * if a file has the Oracle template I will replace it, regardless. We'll think of the user-facing impact later. * for templates without an existing Oracle license header, I will not add a license header but exclude it from Rat. The only exception where I would add the license header is category 2 where it is clear the end user does not get to see or edit the template. But we have to prove this for each individual file and it's not really worth the trouble because these templates are simple (not much Intelectual Property, really) and the lack of a license header does not imply Public Domain anyhow (the whole of NetBeans does have a license). I don't think it's up to us to teach developers copyright and patent law. A lot of developers are really clueless about the legal ramifications and they will dislike a lot seeing Apache license headers in "their" code generation pipeline (because, aren't they creating derivative works? Actually, they are!). By the time you have them thinking about this (or, maybe even, ask their own legal counsel) you have already lost a customer. ---
[GitHub] incubator-netbeans issue #52: [NETBEANS-54] Module review xml modules
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/52 @matthiasblaesing: so far I have been excluding template files from Rat. They are user visible as far as I can tell and I don't believe it's good for NetBeans to have that. Maybe there is some flag or something in the non-dev builds that hide the licese... I have yet to see the license header disappear when shown. ---
[GitHub] incubator-netbeans pull request #44: Testcases related fixes
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/44#discussion_r143385771
--- Diff:
autoupdate.services/test/unit/src/org/netbeans/api/autoupdate/UpdateProviderFactoryCreateTest.java
---
@@ -50,7 +49,7 @@ protected void tearDown () throws Exception {
public void testCreate () throws Exception {
String name = "new-one";
String displayName = "Newone";
-URL url = UpdateUnitFactoryTest.class.getResource
("data/catalog.xml");
+URL url = getClass().getResource
("/org/netbeans/modules/autoupdate/updateprovider/data/catalog.xml");
--- End diff --
I didn't know you were waiting for me. Sure, it looks good!
---
[GitHub] incubator-netbeans issue #95: [NETBEANS-54] Module Review apisupport.ant
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/95 > Yes, though did them just to get it done. But if the conclusion is that these should *not* have headers, we'll have to go back and delete them. ---
[GitHub] incubator-netbeans issue #52: [NETBEANS-54] Module review xml modules
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/52 > I disagree with your assessment about the license header for the templates. I draw the line between the template and its result. I believe we should discuss this on the dev@ mailing lists. Personally I see it as a bad move to scare users that generated code might be Apache-licensed. ---
[GitHub] incubator-netbeans issue #72: Updating license header using the updated lice...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/72 Looks good to me, please commit. ---
[GitHub] incubator-netbeans issue #64: [NETBEANS-54] Module Review j2ee.core.utilitie...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/64 Perfect! ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143060563 --- Diff: j2ee.core.utilities/src/org/netbeans/modules/j2ee/core/support/java/method/ExceptionsPanel.form --- @@ -1,26 +1,5 @@ -
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058559 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -35,20 +56,20 @@ -For list of available properties check the imported -nbproject/build-impl.xml file. --- End diff -- More whitespace. ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058376 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -1,4 +1,25 @@ +
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058744 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/nbproject/project.properties --- @@ -1,3 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance --- End diff -- OK ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058582 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -35,20 +56,20 @@ -For list of available properties check the imported -nbproject/build-impl.xml file. +For list of available properties check the imported +nbproject/build-impl.xml file. Another way to customize the build is by overriding existing main targets. -The targets of interest are: +The targets of interest are: --- End diff -- More whitespace. ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058725 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/nbproject/build-impl.xml --- @@ -21,9 +41,9 @@ is divided into following sections: --> http://www.netbeans.org/ns/j2se-project/1; xmlns:j2seproject2="http://www.netbeans.org/ns/j2se-project/2; xmlns:j2seproject3="http://www.netbeans.org/ns/j2se-project/3; xmlns:jaxrpc="http://www.netbeans.org/ns/j2se-project/jax-rpc; basedir=".." default="default" name="JavaApp-impl"> -
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058252 --- Diff: j2ee.core.utilities/src/org/netbeans/modules/j2ee/core/support/java/method/ExceptionsPanel.form --- @@ -1,26 +1,5 @@ -
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058527 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -7,9 +28,9 @@
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058623 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -35,20 +56,20 @@ -For list of available properties check the imported -nbproject/build-impl.xml file. +For list of available properties check the imported +nbproject/build-impl.xml file. Another way to customize the build is by overriding existing main targets. -The targets of interest are: +The targets of interest are: -init-macrodef-javac: defines macro for javac compilation -init-macrodef-junit: defines macro for junit execution -init-macrodef-debug: defines macro for class debugging -init-macrodef-java: defines macro for class execution -do-jar-with-manifest:JAR building (if you are using a manifest) -do-jar-without-manifest: JAR building (if you are not using a manifest) - run: execution of project + run: execution of project --- End diff -- More whitespace. ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058793 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/nbproject/project.xml --- @@ -1,4 +1,24 @@ +
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058704 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/nbproject/build-impl.xml --- @@ -1,5 +1,25 @@
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058683 --- Diff: j2ee.core.utilities/test/unit/data/JavaApp/build.xml --- @@ -60,10 +81,10 @@ -Notice that the overridden target depends on the jar target and not only on -the compile target as the regular run target does. Again, for a list of available +Notice that the overridden target depends on the jar target and not only on --- End diff -- More whitespace. ---
[GitHub] incubator-netbeans pull request #31: [NETBEANS-54] Module Review j2ee.core.u...
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/31#discussion_r143058334 --- Diff: j2ee.core.utilities/src/org/netbeans/modules/j2ee/core/support/java/method/ParametersPanel.form --- @@ -1,26 +1,5 @@ -
[GitHub] incubator-netbeans pull request #58: [NETBEANS-54] Module Review parsing.nb
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/58 ---
[GitHub] incubator-netbeans pull request #60: [NETBEANS-54] Module Review spellchecke...
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/60 [NETBEANS-54] Module Review spellchecker You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-spellchecker Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/60.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #60 ---
[GitHub] incubator-netbeans pull request #58: [NETBEANS-54] Module Review parsing.nb
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/58 [NETBEANS-54] Module Review parsing.nb You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-parsing.nb Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/58.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #58 commit c02b9bf06c09175101ea8385eb2dfdd2c30a4dcc Author: Emilian Bold <e...@apache.org> Date: 2017-10-04T14:36:52Z [NETBEANS-54] Module Review parsing.nb ---
[GitHub] incubator-netbeans pull request #57: [NETBEANS-54] Module Review parsing.api
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/57 [NETBEANS-54] Module Review parsing.api - No external binaries. - Updated licenses in arch.xml - No additional problems found. You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-parsing.api Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/57.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #57 commit e2c63ff9476dd977647dc87dd3a84777816503b9 Author: Emilian Bold <e...@apache.org> Date: 2017-10-04T14:29:46Z [NETBEANS-54] Module Review parsing.api - No external binaries. - Updated licenses in arch.xml - No additional problems found. ---
[GitHub] incubator-netbeans pull request #44: Testcases related fixes
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/44#discussion_r142631994
--- Diff:
autoupdate.services/test/unit/src/org/netbeans/api/autoupdate/UpdateProviderFactoryCreateTest.java
---
@@ -50,7 +49,7 @@ protected void tearDown () throws Exception {
public void testCreate () throws Exception {
String name = "new-one";
String displayName = "Newone";
-URL url = UpdateUnitFactoryTest.class.getResource
("data/catalog.xml");
+URL url = getClass().getResource
("/org/netbeans/modules/autoupdate/updateprovider/data/catalog.xml");
--- End diff --
Doesn't the relative path data/catalog.xml work anymore?
---
[GitHub] incubator-netbeans issue #47: [NETBEANS-54] Module Review c.jcraft.jsch
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/47 You know, I was just thinking how super useful it is to have the hash when switching to Maven Central since at least you know it's the same JAR. How could they differ for the same library? ---
[GitHub] incubator-netbeans issue #47: [NETBEANS-54] Module Review c.jcraft.jsch
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/47 These look good to me. Before doing the final push you might do: ---
[GitHub] incubator-netbeans issue #47: [NETBEANS-54] Module Review c.jcraft.jsch
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/47 @junichi11 will you commit this yourself? ---
[GitHub] incubator-netbeans issue #41: [NETBEANS-54] Module Review c.google.guava
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/41 Feel free to commit. ---
[GitHub] incubator-netbeans pull request #44: Testcases related fixes
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/44#discussion_r142446465
--- Diff:
autoupdate.services/test/unit/src/org/netbeans/api/autoupdate/UpdateProviderFactoryCreateTest.java
---
@@ -50,7 +50,7 @@ protected void tearDown () throws Exception {
public void testCreate () throws Exception {
String name = "new-one";
String displayName = "Newone";
-URL url = UpdateUnitFactoryTest.class.getResource
("data/catalog.xml");
+URL url = CatalogCacheTest.class.getResource ("data/catalog.xml");
--- End diff --
Why not use `UpdateProviderFactoryCreateTest.class` here? Or even
`getClass().getResource()` ?
---
[GitHub] incubator-netbeans issue #14: [NETBEANS-54] Module Review o.n.swing.tabcontr...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/14 Merged. ---
[GitHub] incubator-netbeans pull request #14: [NETBEANS-54] Module Review o.n.swing.t...
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/14 ---
[GitHub] incubator-netbeans issue #17: [NETBEANS-54] Module Review openide.text
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/17 Committed. ---
[GitHub] incubator-netbeans pull request #18: [NETBEANS-54] Module Review settings
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/18 ---
[GitHub] incubator-netbeans issue #18: [NETBEANS-54] Module Review settings
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/18 Committed. ---
[GitHub] incubator-netbeans issue #25: NETBEANS-73: Autodetect id_rsa and id_dsa keys...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/25 Please close this PR. ---
[GitHub] incubator-netbeans issue #25: NETBEANS-73: Autodetect id_rsa and id_dsa keys...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/25 Committed: https://github.com/apache/incubator-netbeans/commit/ff5f3c4e175cf15c1b4b0caebfb5047be4196a44 ---
[GitHub] incubator-netbeans issue #33: -external library jna-platform-4.2.2.jar: dual...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/33 I don't believe there is much IP value in the binaries-list files and I would not add that huge license header in all of them when they generally have 1-2 lines. The overall project license applies to all the files regardless. So my solution would be not to add the license header to the binaries-list files and just add a RAT exclusion filter. ---
[GitHub] incubator-netbeans issue #33: -external library jna-platform-4.2.2.jar: dual...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/33 The way I understood it, I will absolutely not touch "Problems to be solved centrally" because they will be dealt with entirely at some future moment. Only reason I would care about those is if I see an Oracle copyright header. Not if they have none. > So I was adding the headers to binaries-lists I was modifying, as that should not be wrong either way. It kinda makes sense since you also modified the file, although not substantially (it's still the same dependency) but I would have kept it without a header because it's trivial to add them "centrally" afterwards. ---
[GitHub] incubator-netbeans issue #33: -external library jna-platform-4.2.2.jar: dual...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/33 So, are we adding license to binaries-list? Isn't is part of the "Problems to be solved centrally"? ---
[GitHub] incubator-netbeans issue #12: properties for by module report
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/12 I don't believe you are rebasing correctly. You shouldn't see something like: >jlahoda committed with ebarboni 8 days ago ---
[GitHub] incubator-netbeans issue #11: Fixes bug in FileChooserBuilderTest
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/11 Committed. ---
[GitHub] incubator-netbeans pull request #11: Fixes bug in FileChooserBuilderTest
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/11 ---
[GitHub] incubator-netbeans pull request #19: [NETBEANS-54] Module Review openide.nod...
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/19 ---
[GitHub] incubator-netbeans issue #19: [NETBEANS-54] Module Review openide.nodes
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/19 Committed. ---
[GitHub] incubator-netbeans issue #24: [NETBEANS-54] Module Review db.sql.visualedito...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/24 ... unless the central decision will be not to use headers and then your file becomes the only one that has to be manually tweaked back. ---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/23#discussion_r142003704
--- Diff:
db.sql.editor/test/unit/src/org/netbeans/modules/db/sql/editor/completion/SelectCompletionQueryTest.java
---
@@ -125,11 +125,11 @@ public void runTest() throws Exception {
public void testCompletion() throws Exception {
StringBuilder sqlData = new StringBuilder();
-List modelData = new ArrayList();
-BufferedReader reader = new BufferedReader(new
InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() +
".test").openStream(), "utf-8"));
-try {
+List modelData = new ArrayList<>();
+try (InputStream is =
SelectCompletionQueryTest.class.getResourceAsStream(getName() + ".test");
+BufferedReader reader = new BufferedReader(new
InputStreamReader(is, "utf-8"))) {
boolean separatorRead = false;
-for (String line; (line = reader.readLine()) != null;) {
+for (String line = reader.readLine(); line != null; line =
reader.readLine()) {
--- End diff --
I believe the canonical trick is to have
```java
String line;
while((line = reader.readLine()) != null) {
...
}
```
And the original `for` just mixed those to limit the scope of the `line`
variable to the `for` block... which is nice.
Your `for` repeats the `reader.readLine()` call which seems confusing.
---
[GitHub] incubator-netbeans issue #12: properties for by module report
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/12 The file doesn't seem formatted properly (`<property` and `<rat` look off by 1 space) but other than that it's nice. ---
[GitHub] incubator-netbeans issue #15: [NETBEANS-54] Module Review uihandler
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/15 +1 ---
[GitHub] incubator-netbeans pull request #16: [NETBEANS-54] Module Review db.dataview
Github user emilianbold commented on a diff in the pull request: https://github.com/apache/incubator-netbeans/pull/16#discussion_r141999524 --- Diff: db.dataview/test/unit/src/org/netbeans/modules/db/dataview/util/TestCaseDataFactory.java --- @@ -39,11 +39,11 @@ public static String DB_SQLINSERT="dbinsert.sql"; public static String DB_SQLSELECT="dbselect.sql"; public static String DB_SQLUPDATE="dbupdate.sql"; -public static String DB_TEXT= "dbdata.txt"; +public static String DB_DATA= "dbdata.properties"; --- End diff -- I don't understand why this was necessary. Just so we can add comments? ---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/23#discussion_r141999337
--- Diff:
db.sql.editor/test/unit/src/org/netbeans/modules/db/sql/editor/completion/SelectCompletionQueryTest.java
---
@@ -125,11 +125,11 @@ public void runTest() throws Exception {
public void testCompletion() throws Exception {
StringBuilder sqlData = new StringBuilder();
-List modelData = new ArrayList();
-BufferedReader reader = new BufferedReader(new
InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() +
".test").openStream(), "utf-8"));
-try {
+List modelData = new ArrayList<>();
+try (InputStream is =
SelectCompletionQueryTest.class.getResourceAsStream(getName() + ".test");
+BufferedReader reader = new BufferedReader(new
InputStreamReader(is, "utf-8"))) {
boolean separatorRead = false;
-for (String line; (line = reader.readLine()) != null;) {
+for (String line = reader.readLine(); line != null; line =
reader.readLine()) {
--- End diff --
Wasn't the previous `for` line better?
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/23#discussion_r141999350
--- Diff:
db.sql.editor/test/unit/src/org/netbeans/modules/db/sql/editor/completion/SelectCompletionQueryTest.java
---
@@ -143,27 +143,19 @@ public void testCompletion() throws Exception {
}
}
}
-} finally {
-reader.close();
}
String sql = sqlData.toString();
Metadata metadata = TestMetadata.create(modelData);
if (stdout) {
performTest(sql, metadata, System.out);
} else {
File result = new File(getWorkDir(), getName() + ".result");
-Writer writer = new BufferedWriter(new OutputStreamWriter(new
FileOutputStream(result), "utf-8"));
-try {
+try (Writer writer = new BufferedWriter(new
OutputStreamWriter(new FileOutputStream(result), "utf-8"))) {
performTest(sql, metadata, writer);
-} finally {
-writer.close();
}
File pass = new File(getWorkDir(), getName() + ".pass");
-InputStream input =
SelectCompletionQueryTest.class.getResource(getName() + ".pass").openStream();
-try {
-copyStream(input, pass);
-} finally {
-input.close();
+try (InputStream input =
SelectCompletionQueryTest.class.getResource(getName() + ".pass").openStream()) {
--- End diff --
Changing this was not necessary. But if we do it, why not
`getResourceAsStream`?
---
[GitHub] incubator-netbeans issue #24: [NETBEANS-54] Module Review db.sql.visualedito...
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/24 binaries-list is a "problem to be solved centrally". I would not add license headers to it. ---
[GitHub] incubator-netbeans pull request #25: NETBEANS-73: Autodetect id_rsa and id_d...
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/25#discussion_r141999246
--- Diff:
git/src/org/netbeans/modules/git/ui/repository/remote/RemoteRepository.java ---
@@ -686,6 +686,7 @@ public SSHConnectionSettingsType () {
settingsPanel.savePasswordCheckBox
};
acceptableSchemes = EnumSet.of(Scheme.SSH, Scheme.SFTP);
+
settingsPanel.txtIdentityFile.setText(getDefaultIdentityFilePath());
--- End diff --
This line does not seem necessary.
`SSHConnectionSettingsType.populateFields` does call
```java
String identityFile = getDefaultIdentityFilePath();
settingsPanel.txtIdentityFile.setText(identityFile);
```
---
[GitHub] incubator-netbeans pull request #25: NETBEANS-73: Autodetect id_rsa and id_d...
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/25#discussion_r141999265
--- Diff:
git/src/org/netbeans/modules/git/ui/repository/remote/RemoteRepository.java ---
@@ -853,9 +854,13 @@ protected int getPreferedPanelHeight () {
private String getDefaultIdentityFilePath () {
String identityFile = ""; //NOI18N
-if (!Utilities.isWindows()) {
-identityFile = System.getProperty("user.home") +
File.separator //NOI18N
-+ ".ssh" + File.separator + "id_dsa"; //NOI18N
+File sshDir = new File(System.getProperty("user.home"),
".ssh"); //NOI18N
+File rsaKey = new File(sshDir, "id_rsa"); //NOI18N
+File dsaKey = new File(sshDir, "id_dsa"); //NOI18N
+if (rsaKey.canRead()) {
+identityFile = rsaKey.getAbsolutePath();
+} else if (dsaKey.canRead()) {
+identityFile = dsaKey.getAbsolutePath();
--- End diff --
This looks OK.
---
[GitHub] incubator-netbeans issue #27: [NETBEANS-54] Module Review api.xml.ui
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/27 Some extra newlines at the end but looks ok. ---
[GitHub] incubator-netbeans issue #19: [NETBEANS-54] Module Review openide.nodes
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/19 Looks like a Jackpot hints file. Not sure when it's loaded for tests only though... ---
[GitHub] incubator-netbeans pull request #9: [NETBEANS-54] Module Review openide.util...
Github user emilianbold closed the pull request at: https://github.com/apache/incubator-netbeans/pull/9 ---
[GitHub] incubator-netbeans pull request #19: [NETBEANS-54] Module Review openide.nod...
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/19 [NETBEANS-54] Module Review openide.nodes You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-openide.nodes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/19.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #19 ---
[GitHub] incubator-netbeans pull request #18: [NETBEANS-54] Module Review settings
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/18 [NETBEANS-54] Module Review settings You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-settings Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/18.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #18 commit ff62698079f4e628618550dc33e84e51b09f4727 Author: Emilian Bold <e...@apache.org> Date: 2017-09-27T09:13:15Z [NETBEANS-54] Module Review settings ---
[GitHub] incubator-netbeans pull request #17: [NETBEANS-54] Module Review openide.tex...
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/17 [NETBEANS-54] Module Review openide.text You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-openide.text Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/17.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #17 commit 67c571bcca67f425fecc6814bfbc8d94264ebb0b Author: Emilian Bold <e...@apache.org> Date: 2017-09-27T08:44:36Z [NETBEANS-54] Module Review openide.text ---
[GitHub] incubator-netbeans pull request #11: Fixes bug in FileChooserBuilderTest
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/11 Fixes bug in FileChooserBuilderTest The tmpdir is loaded but the temporary file is not created in it, resulting on macOS in a Permission denied IOException because java cannot write on root (/). The fix just places the temporary file in java.io.tmpdir as it was probably intended. You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-bugfix-openide-filesystems-nb Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/11.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #11 commit 874bb58b59c43426c43088057159175dd47ef7ea Author: Emilian Bold <e...@apache.org> Date: 2017-09-26T08:57:33Z Fixes bug in FileChooserBuilderTest The tmpdir is loaded but the temporary file is not created in it, resulting on macOS in a Permission denied IOException because java cannot write on root (/). The fix just places the temporary file in java.io.tmpdir as it was probably intended. ---
[GitHub] incubator-netbeans pull request #10: [NETBEANS-54] Module Review openide.fil...
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/10 [NETBEANS-54] Module Review openide.filesystems.* * openide.filesystems * openide.filesystems.nb * openide.filesystems.compat8 (no change) You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-openide-filesystems Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/10.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #10 commit 9fce19e5c1313c90e20fbb7e971e474f98422463 Author: Emilian Bold <e...@apache.org> Date: 2017-09-26T08:45:27Z [NETBEANS-54] Module Review openide.filesystems commit 2553bf5e50d075d89eed3531587c4da8f782e9a1 Author: Emilian Bold <e...@apache.org> Date: 2017-09-26T08:48:42Z [NETBEANS-54] Module Review openide.filesystems.nb ---
[GitHub] incubator-netbeans pull request #9: [NETBEANS-54] Module Review openide.util...
GitHub user emilianbold opened a pull request: https://github.com/apache/incubator-netbeans/pull/9 [NETBEANS-54] Module Review openide.util.* This branch covers: * openide.util : no changes * openide.util.enumerations : no changes * openide.util.lookup : see patches / wiki * openide.util.ui: see patches / wiki You can merge this pull request into a Git repository by running: $ git pull https://github.com/emilianbold/incubator-netbeans emi-review-openide-util Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-netbeans/pull/9.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #9 commit 603618ed73216e5b1ca84744372f15821f2f1520 Author: Emilian Bold <e...@apache.org> Date: 2017-09-26T08:04:07Z [NETBEANS-54] Module Review openide.util.ui Adds some license headers to test resources. Minor change in UtilitiesTranslateTest to support comment lines (ie. the license header) for a given test resource. commit 557faa75fe80bfc57e6b19ea02696ec8bf601a66 Author: Emilian Bold <e...@apache.org> Date: 2017-09-26T08:05:24Z [NETBEANS-54] Module Review openide.util.lookup ---
[GitHub] incubator-netbeans issue #8: [NETBEANS-54] Module Review api.htmlui
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/8 @asfgit who are you? ---
[GitHub] incubator-netbeans issue #3: NETBEANS-59 - Document split actions
Github user emilianbold commented on the issue: https://github.com/apache/incubator-netbeans/pull/3 Of course, it's much easier to read since there are no more newline changes. What this patch seems to do is add an @ActionReference so we can have shortcuts for those 3 actions. So, as a fix I would have preferred 2 commits: * one fixing in-place the existing actions (in order to see the actual meat) and * another commit refactoring the classes. I seems the existing static inner classes might have been sufficient: you could have just made them public. You could have introduced SplitDocumentHorizontallyAction which just extends SplitDocumentAction and calls super(tc, JSplitPane.HORIZONTAL_SPLIT). Same for SplitDocumentVerticallyAction. I believe initTopComponent only exists so you can have a no-arg constructor which is odd because you are instantiating the actions manually (so it must have to do with the registrations). Also, you know that `(tc instanceof Splitable && ((Splitable)tc).canSplit())` before it's called. So, what I am curious to learn is how does the shortcut work for your manually created instance? ---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139607093
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -71,7 +74,19 @@ private NbAuthenticator() {
static void install() {
if (Boolean.valueOf(NbBundle.getMessage(GuiRunLevel.class,
"USE_Authentication"))) {
-setDefault(new NbAuthenticator());
+// Look for custom authenticator
+Authenticator authenticator =
Lookup.getDefault().lookup(Authenticator.class);
+if (authenticator == null) {
+authenticator = new NbAuthenticator();
+}
+if (authenticator.getClass().equals(NbAuthenticator.class)) {
--- End diff --
I assumed only a Platform application would need that. I didn't think that
people might want to install a 3rd party plugin in their existing IDE/Platform
app.
---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139602545
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -71,7 +74,19 @@ private NbAuthenticator() {
static void install() {
if (Boolean.valueOf(NbBundle.getMessage(GuiRunLevel.class,
"USE_Authentication"))) {
-setDefault(new NbAuthenticator());
+// Look for custom authenticator
+Authenticator authenticator =
Lookup.getDefault().lookup(Authenticator.class);
+if (authenticator == null) {
+authenticator = new NbAuthenticator();
+}
+if (authenticator.getClass().equals(NbAuthenticator.class)) {
--- End diff --
Because it's not usual to log which instance you are using from the
`Lookup`. Since this is a static situation, you know at build-time which
instance is going to be used, unless somebody yanks your whole module with the
other implementation out.
---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139602349
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -61,6 +63,7 @@
*/
final class NbAuthenticator extends java.net.Authenticator {
--- End diff --
Ah, I see, and `NbAuthenticator` is package private too. I guess this is
just standard NetBeans architecture of not over-exposing more than necessary.
I guess:
* we could either make `NbAuthenticator` class and constructor public or
* introduce a `AuthenticatorFactory` which will be registered in the lookup
with a default factory producing `NbAuthenticator` or
* use your current solution
---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139596801
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -71,7 +74,19 @@ private NbAuthenticator() {
static void install() {
if (Boolean.valueOf(NbBundle.getMessage(GuiRunLevel.class,
"USE_Authentication"))) {
-setDefault(new NbAuthenticator());
--- End diff --
... then just call
`setDefault(Lookup.getDefault().lookup(Authenticator.class))` here.
There will always be at least one instance in the lookup and the other
implementations can jump in front with the `position` attribute.
---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139596868
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -71,7 +74,19 @@ private NbAuthenticator() {
static void install() {
if (Boolean.valueOf(NbBundle.getMessage(GuiRunLevel.class,
"USE_Authentication"))) {
-setDefault(new NbAuthenticator());
+// Look for custom authenticator
+Authenticator authenticator =
Lookup.getDefault().lookup(Authenticator.class);
+if (authenticator == null) {
+authenticator = new NbAuthenticator();
+}
+if (authenticator.getClass().equals(NbAuthenticator.class)) {
--- End diff --
Everything else, including the log call doesn't seem necessary.
---
[GitHub] incubator-netbeans pull request #2: Allow custom authenticator
Github user emilianbold commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/2#discussion_r139596689
--- Diff: o.n.core/src/org/netbeans/core/NbAuthenticator.java ---
@@ -61,6 +63,7 @@
*/
final class NbAuthenticator extends java.net.Authenticator {
--- End diff --
Why not just add `@ServiceProvider(service= Authenticator.class)` here?
---
