Hi Jonathan, just wondering if you still plan to look at offering PRs for:
https://issues.apache.org/jira/browse/TINKERPOP-998
https://issues.apache.org/jira/browse/TINKERPOP-997
I'll stay away from those, if you think you will be working on them.
On Mon, Nov 30, 2015 at 1:39 PM, Stephen Mallette <spmalle...@gmail.com>
wrote:
> > Using VertexProperyFeatures.FEATURE_{ADD, REMOVE}_PROPERTY perhaps
> would be more consistent with the logic used everywhere else...
>
> yeah - i'm +1 for this approach. it makes more sense given ADD/REMOVE
> already being the pattern for graph Element instances.
>
> On Mon, Nov 30, 2015 at 1:31 PM, Jonathan Ellithorpe <j...@cs.stanford.edu>
> wrote:
>
>> 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
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
