You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by stain <gi...@git.apache.org> on 2016/11/07 01:28:17 UTC
[GitHub] jena pull request #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
GitHub user stain opened a pull request:
https://github.com/apache/jena/pull/189
JENA-1258 NodeFmtLib calls JenaSystem initializer
Fixes [JENA-1258](https://issues.apache.org/jira/browse/JENA-1258) in `NodeFmtLib`
Note that the added `TestNodeFmtInit` test case would not normally find the bug ran from Maven as earlier tests would have caused initialization, but it can be tested from say Eclipse.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stain/jena JENA-1258
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/189.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 #189
----
commit 170f7e7f1e9634b3afae0a6bd14fc7c88f792b7b
Author: Stian Soiland-Reyes <st...@apache.org>
Date: 2016-11-07T01:20:25Z
JENA-1258 NodeFmtLib initializer for JenaSystem.init
----
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/189
The better fix is to initialize in `NodeFactory` (and `Node` for legacy reasons) only. I have tested this in a build.
There is little point in a specific test because tests run in the overall suite; it's almost confusing to have a test that doesn't. With initialization in `NodeFactory` this will get tested anyway!
Also, it would help to unroll ARQConstants (for RDF, RDFS, OWL, XSD) so it does not need JenaSystem.init and then it is, with care, init-safe.
See PR#192 for that all made real.
The original example works when run separately to ensure minimal system initialization.
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/189
This usage is calling into the middle of internal code.
Fixing output code seems wrong. It is better to initialize as objects get created, not on output. This shows the root cause is somewhere else.
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/189
The root cause:
Commit 204f253 switched Context to using a `ConcurrentHashMap`, commit d67a426 added a check for null otherwise nothing has changed since 3.1.0.
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by stain <gi...@git.apache.org>.
Github user stain commented on the issue:
https://github.com/apache/jena/pull/189
Except for `impl` packages and suffixes it is never easy for Jena users to know what is "internal code".
If it's in [Javadoc](https://jena.apache.org/documentation/javadoc/arq/org/apache/jena/riot/out/NodeFmtLib.html), and it is useful "outside" without much hazzle, then I don't see why it should be called "internal" just because it is also used internally.
Agree that it would be good to ensure Jena initialization from `NodeFactory`.
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by stain <gi...@git.apache.org>.
Github user stain commented on the issue:
https://github.com/apache/jena/pull/189
Yes, your pull request looks like the best approach.
On 8 Nov 2016 12:33 pm, "Andy Seaborne" <no...@github.com> wrote:
> The better fix is to initialize in NodeFactory (and Node for legacy
> reasons) only. I have tested this in a build.
>
> There is little point in a specific test because tests run in the overall
> suite; it's almost confusing to have a test that doesn't. With
> initialization in NodeFactory this will get tested anyway!
>
> Also, it would help to unroll ARQConstants (for RDF, RDFS, OWL, XSD) so it
> does not need JenaSystem.init and then it is, with care, init-safe.
>
> See PR#192 for that all made real.
>
> The original example works when run separately to ensure minimal system
> initialization.
>
> \u2014
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jena/pull/189#issuecomment-259124635>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAPd5fFN_RTsft_0zoh3lFvprRyNmYrsks5q8Gu3gaJpZM4Kqwha>
> .
>
---
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 #189: JENA-1258 NodeFmtLib calls JenaSystem initializer
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/189
---
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.
---