You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ajs6f <gi...@git.apache.org> on 2016/10/08 19:13:50 UTC
[GitHub] jena pull request #176: Minor cleanup
GitHub user ajs6f opened a pull request:
https://github.com/apache/jena/pull/176
Minor cleanup
Just removing dead code and variables that don't get used, adding `static` here or there to clarify tests a bit, that sort of thing. Pruning.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajs6f/jena MinorCleanup
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/176.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 #176
----
commit f3a18f0de16bc0a66111535ad16514f28f1f71db
Author: ajs6f <aj...@virginia.edu>
Date: 2016-10-08T18:55:36Z
Minor cleanup and removing dead code
commit 50983a6a9de2f05f10acc9cc985cf53eba1529ac
Author: ajs6f <aj...@virginia.edu>
Date: 2016-10-08T19:12:26Z
Small cleanups in test code
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510619
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -162,10 +162,6 @@
}
}
- private static boolean emptyDir(Path dir) {
--- End diff --
Move down to near `mustExist`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/176
Can this wait until after the 3.1.1 release?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84577738
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/mgt/ActionDatasets.java ---
@@ -111,8 +110,6 @@ protected JsonValue execGetItem(HttpAction action) {
@Override
protected JsonValue execPostContainer(HttpAction action) {
JenaUUID uuid = JenaUUID.generate() ;
--- End diff --
I sometimes suspect that when software projects grow old and complex enough, they stop being deterministic. {grin}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
Oh, cool. Okay, back to "will merge EOD today unless @Claudenw objects".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84549447
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/mgt/ActionDatasets.java ---
@@ -111,8 +110,6 @@ protected JsonValue execGetItem(HttpAction action) {
@Override
protected JsonValue execPostContainer(HttpAction action) {
JenaUUID uuid = JenaUUID.generate() ;
--- End diff --
You are right. Don't why it didn't cause a compile error when I tried deleting it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511824
--- Diff: jena-permissions/src/main/java/org/apache/jena/permissions/impl/SecuredItemImpl.java ---
@@ -312,7 +312,7 @@ private Boolean cacheGet(final CacheKey key) {
* @param value
* The value to set.
*/
- private void cachePut(final CacheKey key, final boolean value) {
+ private static void cachePut(final CacheKey key, final boolean value) {
--- End diff --
See earlier comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510977
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java ---
@@ -74,7 +74,7 @@ public QueryIterTopN(QueryIterator qIter, Comparator<Binding> comparator, long n
}
// Keep heap with maximum accessible.
- this.heap = new PriorityQueue<Binding>((int)numItems, new ReverseComparator<Binding>(comparator)) ;
+ this.heap = new PriorityQueue<>((int)numItems, new ReverseComparator<>(comparator)) ;
--- End diff --
Right-- got to catch up with my own changes!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511004
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -371,19 +367,6 @@ else if ( ! dir.isDirectory())
throw new FusekiConfigException("Not a directory: "+directory) ;
}
- private static void mustExist(Path directory) {
- File dir = directory.toFile() ;
- if ( ! dir.exists() )
- throw new FusekiConfigException("Does not exist: "+directory) ;
- if ( ! dir.isDirectory())
- throw new FusekiConfigException("Not a directory: "+directory) ;
- }
-
- private static boolean exists(Path directory) {
--- End diff --
All in a group at the end.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84518062
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/mgt/ActionDatasets.java ---
@@ -111,8 +110,6 @@ protected JsonValue execGetItem(HttpAction action) {
@Override
protected JsonValue execPostContainer(HttpAction action) {
JenaUUID uuid = JenaUUID.generate() ;
--- End diff --
This can go as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/176
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84510072
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java ---
@@ -53,7 +53,7 @@ public AsyncTask submit(Runnable task, String displayName, DataService dataServi
Fuseki.serverLog.info(format("Task : %s : %s",taskId, displayName)) ;
Callable<Object> c = Executors.callable(task) ;
AsyncTask asyncTask = new AsyncTask(c, this, taskId, displayName, dataService, requestId) ;
- Future<Object> x = executor.submit(asyncTask) ;
+ executor.submit(asyncTask) ;
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510610
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java ---
@@ -53,7 +53,7 @@ public AsyncTask submit(Runnable task, String displayName, DataService dataServi
Fuseki.serverLog.info(format("Task : %s : %s",taskId, displayName)) ;
Callable<Object> c = Executors.callable(task) ;
AsyncTask asyncTask = new AsyncTask(c, this, taskId, displayName, dataService, requestId) ;
- Future<Object> x = executor.submit(asyncTask) ;
+ executor.submit(asyncTask) ;
--- End diff --
I prefer the assignment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510645
--- Diff: jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/FileSender.java ---
@@ -79,8 +79,8 @@ public void send(String method) {
}
ps.print("--" + boundary + "--"+WNL);
}
- connection.connect();
- int responseCode = connection.getResponseCode();
--- End diff --
I prefer the assignment - helps debugging!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
I will try to fix these things sometime tomorrow in another commit, especially if I can fix #151.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
I think I addressed the comments given here in that last commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
The bulk of changes to `jena-permissions` are using `<>` instead of writing out type parameters, but there are also a few more substantive syntax and other changes there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511001
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -162,10 +162,6 @@
}
}
- private static boolean emptyDir(Path dir) {
--- End diff --
Okay.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510620
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -371,19 +367,6 @@ else if ( ! dir.isDirectory())
throw new FusekiConfigException("Not a directory: "+directory) ;
}
- private static void mustExist(Path directory) {
- File dir = directory.toFile() ;
- if ( ! dir.exists() )
- throw new FusekiConfigException("Does not exist: "+directory) ;
- if ( ! dir.isDirectory())
- throw new FusekiConfigException("Not a directory: "+directory) ;
- }
-
- private static boolean exists(Path directory) {
--- End diff --
3rd of 3
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510973
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/BlockUTF8.java ---
@@ -227,11 +220,6 @@ else if ( ch <= 0x7FFFFFFF )
}
//int bytesFinish = bb.position() ;
}
-
- private static void fromCharsArray(char[] array, byte[] array2)
- {
- throw new NotImplemented() ;
- }
--- End diff --
Will do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by Claudenw <gi...@git.apache.org>.
Github user Claudenw commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511612
--- Diff: jena-permissions/src/main/java/org/apache/jena/permissions/impl/SecuredItemImpl.java ---
@@ -312,7 +312,7 @@ private Boolean cacheGet(final CacheKey key) {
* @param value
* The value to set.
*/
- private void cachePut(final CacheKey key, final boolean value) {
+ private static void cachePut(final CacheKey key, final boolean value) {
--- End diff --
Again, this change modifies the is not semantic meaning.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510652
--- Diff: jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestAdmin.java ---
@@ -309,11 +310,6 @@
}
}
- private JsonValue getTask(String taskId) {
- String url = urlRoot+"$/tasks/"+taskId ;
--- End diff --
A useful verb (reflect lack of code for batch task handling).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510609
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -371,19 +367,6 @@ else if ( ! dir.isDirectory())
throw new FusekiConfigException("Not a directory: "+directory) ;
}
- private static void mustExist(Path directory) {
--- End diff --
Please leave. While unused, these 3 methods form verbs for working with Paths - I'd prefer to leave the verbs around.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
@Claudenw @afs Hang on-- I just reread that @afs would like this to hold until post-3.1.1, so I won't merge it after all.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
A few more commits to come this afternoon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
@Claudenw If you have no further objection, I'm going to merge this EOD today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510444
--- Diff: jena-base/src/test/java/org/apache/jena/atlas/io/AbstractTestPeekInputStream.java ---
@@ -140,7 +139,7 @@
{
PeekInputStream in = make("abc") ;
checkLineCol(in, INIT_LINE, INIT_COL) ;
- int ch = in.readByte() ;
+ in.readByte() ;
--- End diff --
Please leave - assignment shows there is a return. The compiler can sort the rest out (or at least `/* int ch = */` but there is much code that is "unused" already).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
> Unless there is a good reason to make the 2 permissions methods static I would prefer to leave them alone.
I had some reason (explained above) but since seems less clear to you, I will remove it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
Glad to!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510625
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/validation/DataValidator.java ---
@@ -101,33 +95,6 @@ protected JsonObject execute(ValidationAction action) {
protected String validatorName() {
return "RDF Data" ;
}
-
- // Error handler that records messages
- private static class ErrorHandlerMsg implements ErrorHandler
--- End diff --
Please leave.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511008
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/validation/DataValidator.java ---
@@ -101,33 +95,6 @@ protected JsonObject execute(ValidationAction action) {
protected String validatorName() {
return "RDF Data" ;
}
-
- // Error handler that records messages
- private static class ErrorHandlerMsg implements ErrorHandler
--- End diff --
Okay-- maybe we could at least exercise it in a test?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510358
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/BlockUTF8.java ---
@@ -227,11 +220,6 @@ else if ( ch <= 0x7FFFFFFF )
}
//int bytesFinish = bb.position() ;
}
-
- private static void fromCharsArray(char[] array, byte[] array2)
- {
- throw new NotImplemented() ;
- }
--- End diff --
Same as above
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510986
--- Diff: jena-base/src/test/java/org/apache/jena/atlas/lib/TestHex.java ---
@@ -85,7 +85,7 @@ private static void testStr2Val(String str, int expected) {
}
private static void testStr2Val(String str, int start, int length, int expected) {
- int v = Hex.hexStringToInt(str, start, length) ;
+ Hex.hexStringToInt(str, start, length) ;
--- End diff --
Okay.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
Absolutely. Would you rather I just left it here and we can pick it up at the right moment, or close this PR and reopen it post-release?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511000
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/FusekiServer.java ---
@@ -371,19 +367,6 @@ else if ( ! dir.isDirectory())
throw new FusekiConfigException("Not a directory: "+directory) ;
}
- private static void mustExist(Path directory) {
--- End diff --
Okay.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510433
--- Diff: jena-base/src/test/java/org/apache/jena/atlas/lib/TestHex.java ---
@@ -85,7 +85,7 @@ private static void testStr2Val(String str, int expected) {
}
private static void testStr2Val(String str, int start, int length, int expected) {
- int v = Hex.hexStringToInt(str, start, length) ;
+ Hex.hexStringToInt(str, start, length) ;
--- End diff --
Indicates that there is a return.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511823
--- Diff: jena-permissions/src/main/java/org/apache/jena/permissions/impl/SecuredItemImpl.java ---
@@ -299,7 +299,7 @@ public String toString() throws AuthenticationRequiredException {
* @return the value of the security check or <code>null</code> if the value
* has not been cached.
*/
- private Boolean cacheGet(final CacheKey key) {
+ private static Boolean cacheGet(final CacheKey key) {
--- End diff --
I thought it seemed clearer (it indicates that the processing involved is not instance-specific), but if you disagree, that is enough for me. I will edit the PR to remove this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510455
--- Diff: jena-base/src/test/java/org/apache/jena/atlas/lib/cache/TestCacheSimple.java ---
@@ -43,7 +43,6 @@ public void testFixedSize() {
@Test
public void testReplace() {
- final Integer key = 1 ;
final String value1 = "A" ;
--- End diff --
Better - use the key not the constant.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/176
I didn't review jena-permissions or jena-jdbc-* or jena-spatial.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510373
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java ---
@@ -74,7 +74,7 @@ public QueryIterTopN(QueryIterator qIter, Comparator<Binding> comparator, long n
}
// Keep heap with maximum accessible.
- this.heap = new PriorityQueue<Binding>((int)numItems, new ReverseComparator<Binding>(comparator)) ;
+ this.heap = new PriorityQueue<>((int)numItems, new ReverseComparator<>(comparator)) ;
--- End diff --
Deprecation of `ReverseComparator` -- use `Comparator#reversed`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510971
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/BlockUTF8.java ---
@@ -154,11 +152,6 @@ else if ( (x & 0xF8) == 0xF0 )
}
}
}
-
- private static void toCharsArray(byte[] bytes, char[] chars)
- {
- throw new NotImplemented() ;
- }
--- End diff --
Will do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510665
--- Diff: jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java ---
@@ -261,18 +260,10 @@ public void query_describe_conneg() {
}
}
-
- private void execQuery(String queryString, int exceptedRowCount) {
+ private static void execQuery(String queryString, int exceptedRowCount) {
QueryExecution qExec = QueryExecutionFactory.sparqlService(serviceQuery, queryString) ;
ResultSet rs = qExec.execSelect() ;
int x = ResultSetFormatter.consume(rs) ;
assertEquals(exceptedRowCount, x) ;
}
-
- private void execQuery(String queryString, ResultSet expectedResultSet) {
- QueryExecution qExec = QueryExecutionFactory.sparqlService(serviceQuery, queryString) ;
--- End diff --
An action that supports the tests - even if not used at the moment, it shows what is possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82510331
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/BlockUTF8.java ---
@@ -154,11 +152,6 @@ else if ( (x & 0xF8) == 0xF0 )
}
}
}
-
- private static void toCharsArray(byte[] bytes, char[] chars)
- {
- throw new NotImplemented() ;
- }
--- End diff --
Better to implement as `toCharsBuffer(ByteBuffer.wrap(bytes), CharBuffer.wrap(chars))`, leaving as a placeholder for a faster (lower overhead) version in the future.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by Claudenw <gi...@git.apache.org>.
Github user Claudenw commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r82511501
--- Diff: jena-permissions/src/main/java/org/apache/jena/permissions/impl/SecuredItemImpl.java ---
@@ -299,7 +299,7 @@ public String toString() throws AuthenticationRequiredException {
* @return the value of the security check or <code>null</code> if the value
* has not been cached.
*/
- private Boolean cacheGet(final CacheKey key) {
+ private static Boolean cacheGet(final CacheKey key) {
--- End diff --
What is the reasoning for making this a static method? While it is using thread local only it seems to me that this is not semantically the same as a static method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84509798
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java ---
@@ -53,7 +53,7 @@ public AsyncTask submit(Runnable task, String displayName, DataService dataServi
Fuseki.serverLog.info(format("Task : %s : %s",taskId, displayName)) ;
Callable<Object> c = Executors.callable(task) ;
AsyncTask asyncTask = new AsyncTask(c, this, taskId, displayName, dataService, requestId) ;
- Future<Object> x = executor.submit(asyncTask) ;
+ executor.submit(asyncTask) ;
--- End diff --
It returns a result. I prefer to assign it and not leave it hidden.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/176
@Claudenw are you okay with this PR now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/176
Given the delay due to sorting out HttpOp etc, it can go in from my POV.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84518512
--- Diff: jena-base/src/test/java/org/apache/jena/atlas/io/AbstractTestPeekInputStream.java ---
@@ -153,7 +152,7 @@
@Test public void unread3()
{
PeekInputStream in = make("") ;
- int ch = in.readByte() ;
+ in.readByte() ;
assertEquals(-1, in.peekByte()) ;
--- End diff --
`/* int ch = */` Please - consistency. L142 has a change, the rest below don't.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #176: Minor cleanup
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/176#discussion_r84518192
--- Diff: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/validation/DataValidator.java ---
@@ -102,7 +102,8 @@ protected String validatorName() {
return "RDF Data" ;
}
- // Error handler that records messages
+ // Error handler that records messages
--- End diff --
Indent error.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---