spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r726113446
##########
File path: gremlin-core/src/test/resources/graphml/sample.graphml.xml
##########
@@ -0,0 +1,367 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review comment:
I can see the need for a new test file as you are adding a new feature
here and I don't really think we should change the existing GraphML files. As a
suggestion, it would be nice if the new test file simply was a modification of
the existing "modern" data set. We try to keep the datasets consistent when we
can even for test. It's not always possible but in this case I think it would
be easy to copy the tinkerpop-modern.xml file and modify it to include default
values. An easy one would be to include defaults for "lang" and/or "age" which
aren't shared across all vertex labels. For edge defaults I suppose you would
have to fabricate something as all edges have "weight" but at least the base
graph data would be the same. if you have some good reason for sticking with
this data as you have though, please let me know, otherwise it would be nice to
see our more familiar dataset in use here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
