...damn - hot key sent my post too soon - trying again: Hi Jonathan, thanks for bringing this up. It would be nice if we could expand coverage of our test suite by simply improving the way in which features are applied. I was about to suggest a big set of changes when I realized that FeatureRequirementSet.SIMPLE is just defined wrong. It shouldn't have this entry:
addFeatureRequirement.Factory.create(Graph.Features.VertexPropertyFeatures.FEATURE_ADD_PROPERTY, Graph.Features.VertexPropertyFeatures.class)); it should just be: add(FeatureRequirement.Factory.create(Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY, Graph.Features.VertexFeatures.class)); I've created an issue for that to track things: https://issues.apache.org/jira/browse/TINKERPOP3-997 because it is a "breaking" change as it will open up tests and possibly cause existing implementations to fail. If you'd like to submit a PR for this little fix, as you were the reporter for it and as someone who can test it in a way that is currently failing for them, just let me know. As for the this issue: Graph.Features.VertexPropertyFeatures.FEATURE_ADD_PROPERTY <==> Graph.Features.VertexFeatures.FEATURE_META_PROPERTIES - yeah - we need to deprecate one of those as they are the same thing. Not sure if anyone has any preferences on that. in one sense, FEATURE_ADD_PROPERTY is better because it matches the approach for Vertex/Edge. On the other hand, the documentation refers to this feature as "meta-properties". I guess i would go with keeping FEATURE_META_PROPERTIES and deprecating FEATURE_ADD_PROPERTY. I've created an issue as such: https://issues.apache.org/jira/browse/TINKERPOP3-998 If no one has any objections in the next 72 hours (Monday, November 30, 2015 at 7:45am) I'll assume lazy consensus and we can move forward with this one. On Fri, Nov 27, 2015 at 7:35 AM, Stephen Mallette <spmalle...@gmail.com> wrote: > Hi Jonathan, thanks for bringing this up. It would be nice if we could > expand coverage of our test suite by simply improving the way in which > features are applied. I was about to suggest a big set of changes when I > realized that FeatureRequirementSet.SIMPLE is just defined wrong. It > shouldn't have > > addFeatureRequirement.Factory.create(Graph.Features.VertexPropertyFeatures.FEATURE_ADD_PROPERTY, > Graph.Features.VertexPropertyFeatures.class)); > > it should just be: > > > On Mon, Nov 23, 2015 at 7:39 PM, Jonathan Ellithorpe <j...@cs.stanford.edu> > wrote: > >> Hello all, >> >> I am currently working on an experimental implementation of TinkerPop3 on >> an in-memory key-value store called RAMCloud. In the process of running >> the >> unit tests I noticed that turning on support for persistence did not >> trigger any new unit tests in GraphTests. Looking into the matter, I found >> that the unit test that tests this, shouldPersistOnClose, was not >> executing >> because meta properties support is included in its feature requirements, >> but I do not have support for meta properties. Oddly, though, this >> features >> requirement seems to be superfluous, since the test does not utilize meta >> properties. >> >> An orthogonal issue seems to be that >> Graph.Features.VertexPropertyFeatures.FEATURE_ADD_PROPERTY <==> >> Graph.Features.VertexFeatures.FEATURE_META_PROPERTIES >> >> Best, >> Jonathan >> > >
