I think it's either that or change FEATURE_META_PROPERTY to a symmetric
VertexFeatures.FEATURE_{ADD, REMOVE}_METAPROPERTY to pair with
VertexFeatures.FEATURE_{ADD, REMOVE}_PROPERTY.
Using VertexProperyFeatures.FEATURE_{ADD, REMOVE}_PROPERTY perhaps would be
more consistent with the logic used everywhere else...
On Mon, Nov 30, 2015 at 6:30 AM Stephen Mallette <spmalle...@gmail.com>
wrote:
> ugh - mess. maybe we should just keep the add/remove symmetry and
> deprecate FEATURE_META_PROPERTY then.
>
>
> On Sat, Nov 28, 2015 at 4:11 PM, Jonathan Ellithorpe <j...@cs.stanford.edu>
> wrote:
>
> > 1) Yes, I can submit a PR for fixing the SIMPLE feature requirement set.
> > 2) I also agree with deprecating
> > VertexPropertyFeatures.FEATURE_ADD_PROPERTY, but looking at the code I
> > think I realized there is a slight complication here. That is, what to do
> > with VertexPropertyFeatures.FEATURE_REMOVE_PROPERTY. Does
> > VertexFeatures.FEATURE_META_PROPERTIES imply both ADD and REMOVE, or only
> > ADD? In the later case, we would need to leave
> > VertexPropertyFeatures.FEATURE_REMOVE_PROPERTIES. Personally, seeing as
> how
> > VertexFeatures, extending ElementFeatures, has a FEATURE_ADD_PROPERTY and
> > FEATURE_REMOVE_PROPERTY, that the FEATURE_META_PROPERTIES be changed to
> > FEATURE_ADD_METAPROPERTY and FEATURE_REMOVE_METAPROPERTY.
> >
> > Jonathan
> >
> > On Fri, Nov 27, 2015 at 4:55 AM Stephen Mallette <spmalle...@gmail.com>
> > wrote:
> >
> > > ...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
> > > >>
> > > >
> > > >
> > >
> >
>
