Cole-Greer commented on code in PR #3209:
URL: https://github.com/apache/tinkerpop/pull/3209#discussion_r2373401373


##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java:
##########
@@ -175,6 +188,9 @@ public void addProperty(Object key, Object value) {
         if (key instanceof GValue) {
             throw new IllegalArgumentException("GValue cannot be used as a 
property key");
         }
+        if (key instanceof Traversal) {
+            this.integrateChild(((Traversal<?, ?>) key).asAdmin());

Review Comment:
   Historically speaking, no. The main purpose of integrateChild is to ensure 
the child is consistent with the parent (has the correct parent set, both 
traversals merge their side effects, and now they also merge their 
GValueManagers). Historically speaking there hasn't been much concern about the 
state of the child after it has been removed, as it is no longer a concern for 
the TraversalParent. One could argue that the removed child should be given an 
EmptyTraversal as a new parent, however this has never been enforced in the 
past.



-- 
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]

Reply via email to