Jonathan, just wanted to throw a heads up your way so that you're aware of our expecting timing. If all goes as planned, we will head into code freeze for 3.1.1-incubating in about three weeks. If you are still planning on submitting PRs for:
https://issues.apache.org/jira/browse/TINKERPOP3-997 https://issues.apache.org/jira/browse/TINKERPOP3-998 we'd need to see it in that time frame. I don't mean to apply pressure, I just don't want to miss the chance to get these fixes in for 3.1.1-incubating. On Wed, Dec 9, 2015 at 1:24 AM, Jonathan Ellithorpe <j...@cs.stanford.edu> wrote: > Hi Stephen, working on that now, thanks for pinging me on this. > > On Tue, Dec 8, 2015 at 4:48 PM Stephen Mallette <spmalle...@gmail.com> > wrote: > > > 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 > > >> > > > >> > > >> > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > > > > >
