You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@netbeans.apache.org by matthiasblaesing <gi...@git.apache.org> on 2017/09/29 20:23:29 UTC
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
GitHub user matthiasblaesing opened a pull request:
https://github.com/apache/incubator-netbeans/pull/23
[NETBEANS-54] Module Review db.sql.editor
- no external library
- checked Rat report; unrecognized license headers manually changed,
missing headers added, ignored *.form (see central problems) and
SQLExample. The latter file is used in the GUI as a sample and
in addition does not reach a level of creativity.
- skimmed through the module, did not notice additional problems
- Modified tests to allow comments in *.test and *.pass files
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/matthiasblaesing/incubator-netbeans db.sql.editor-review
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-netbeans/pull/23.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 #23
----
commit ae8c59fe2679401cb1c533f52f7c838e848b1533
Author: Matthias Bläsing <mb...@doppel-helix.eu>
Date: 2017-09-29T20:19:13Z
[NETBEANS-54] Module Review db.sql.editor
- no external library
- checked Rat report; unrecognized license headers manually changed,
missing headers added, ignored *.form (see central problems) and
SQLExample. The latter file is used in the GUI as a sample and
in addition does not reach a level of creativity.
- skimmed through the module, did not notice additional problems
- Modified tests to allow comments in *.test and *.pass files
----
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/23#discussion_r142002449
--- 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<String> modelData = new ArrayList<String>();
- BufferedReader reader = new BufferedReader(new InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() + ".test").openStream(), "utf-8"));
- try {
+ List<String> 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 consider the previous one less readable. The previous version mixed initialization, loop advance and checking and these parts are now separated into their corresponding sections.
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by emilianbold <gi...@git.apache.org>.
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<String> modelData = new ArrayList<String>();
- BufferedReader reader = new BufferedReader(new InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() + ".test").openStream(), "utf-8"));
- try {
+ List<String> 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 issue #23: [NETBEANS-54] Module Review db.sql.editor
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on the issue:
https://github.com/apache/incubator-netbeans/pull/23
Thanks for looking into this. I let the changed for loop in place and updated the usage of `getResource().openStream()` to `getResourceAsStream`.
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/23#discussion_r142003802
--- 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<String> modelData = new ArrayList<String>();
- BufferedReader reader = new BufferedReader(new InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() + ".test").openStream(), "utf-8"));
- try {
+ List<String> 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'll revert my change to this loop and then push to master.
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by emilianbold <gi...@git.apache.org>.
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 pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-netbeans/pull/23
---
[GitHub] incubator-netbeans pull request #23: [NETBEANS-54] Module Review db.sql.edit...
Posted by emilianbold <gi...@git.apache.org>.
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<String> modelData = new ArrayList<String>();
- BufferedReader reader = new BufferedReader(new InputStreamReader(SelectCompletionQueryTest.class.getResource(getName() + ".test").openStream(), "utf-8"));
- try {
+ List<String> 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.
---