Conversation
kinow
left a comment
kinow
left a comment
There was a problem hiding this comment.
Looks OK to me. Not sure about one change in a file (syntax), but +1 assuming CI tests pass. Thanks @TelicentPaul !
| // (Abort after commit would trigger the warning.) | ||
| if ( txnResetState.length != x ) | ||
| Log.warn(this, format("Mismatch: state.length = %d, committedLength = %d", txnResetState.length != x)); | ||
| Log.warn(this, format("Mismatch: state.length = %d, committedLength = %d", txnResetState.length, x)); |
There was a problem hiding this comment.
Looks like a copy-pasta bug! Good catch!
| NodeIdType type = NodeIdType.intToEnum(t); | ||
| if ( type == NodeIdType.SPECIAL ) | ||
| throw new TDBException(String.format("Attempt to create a special from a long: 0x%016", v2)); | ||
| throw new TDBException(String.format("Attempt to create a special from a long: 0x%016X", v2)); |
There was a problem hiding this comment.
These two changes were the only ones I was not sure. Can't recall ever using this syntax. The rest of the changes look good!
There was a problem hiding this comment.
It's admittedly an odd one. Without the X the format method doesn't know what it's doing and will throw an UnknownFormatConversionException. With the X it knows that it's hexadecimal, and the preceding numbers tell it how many characters to print.
I had to plug it into a wee online java compiler just to be sure.
public static void main(String[] args) {
System.out.println(String.format("Will print a 16 character hex: 0x%016X", 13L));
System.out.println(String.format("Will throw an exception: 0x%016", 13L));
}
|
The files ARQParser.java and ARQParserTokenManager.java are automatically generated - same for all the pairs like that. JavaCC isn't run in the build. In each directory there a shell script that does several things - clear up old stuff, generate the parser, generate the txt file that can be used to produce complete HTML (jjdoc does not), then some patching up such as adding SuppressWarnings ("unused").
|
|
I'm happy to rollback the changes in the auto-generated files since (a) it's only imports (b) it'll presumably be overridden next time the process runs. Just let me know. |
|
We might as well leave the PR as is. Yes, they get overwritten. Not a lot we can do about that short of an amount more of hackery. "Mostly harmless". 🤷🏻 It is as the comment in the source files says! Looks like the class level SuppressWarnings ("unused") is added manually at least sometimes. IIRC it is too fragile to script exact fixes like unused imports always - javacc has changed its output across versions in the past or if the javacc setup changes (which is very rare). |
|
What is odd is that this PR fixes a couple of things that Eclipse does not pick up and there doesn't seem to be an preference option for.
Bad news is that I tried some other warning settings and found a real issue. Another PR ... |
|
Can I ask that as part of your PR you fix the "configutation" typo in FusekiMain.java? |
#2166 is just changes in jena-core. I've changed my local working copy on an on-going branch (about TDB commands) that is close-ish to jena-fuseki-main. OK? |
3 participants
Removing unused imports, fixes to some incorrect string formatting, correcting a case-sensitive switch option & an infinite recursion.
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.