FlorianHockmann commented on PR #2599:
URL: https://github.com/apache/tinkerpop/pull/2599#issuecomment-2112453850

   If I'm not mistaken, then this PR does two things which are mostly separate 
from each other, right? It extends the type definitions for existing types like 
`Vertex`, `Edge`, and `VertexProperty` and it also adds a new utility method 
`parseVertex` which can be used to create a `Vertex` object where a 
`VertexProperty` is directly a property of the object, e.g., `vertex.name`.
   
   I think it would be good to separate these two things from each other. Could 
you please create issues for them in our JIRA? Our usual development workflow 
is to first have an issue there which describes the bug / proposed improvement 
and afterwards create the PR. This also makes it possible to first discuss the 
issue before actually implementing it.
   
   In this case, I'm not 100% about the added utility method. It sounds to me 
like it's a first step in the direction of the [_Sugar 
Plugin_](https://tinkerpop.apache.org/docs/current/reference/#sugar-plugin) for 
Groovy and I'm not sure whether we should implement such syntactic sugar for 
the GLVs like gremlin-javascript.
   But maybe I'm just missing context why this is useful for Javascript / 
Typescript. You could probably better describe your motivation for this in the 
JIRA issue.
   
   Sorry for the slight push back here in general. Your Typescript improvements 
are more than welcome, but I think they are difficult for us to review since we 
simply lack some experience in this area. That is why I think that following 
this process helps as it makes it easier for other contributors to better 
understand the improvements before reviewing them.


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