Hi @olivertowers, first of all, thanks for working on improving Gremlin.NET!

I'm not sure if you're aware of this but we are currently preparing for the 
release of 3.3.6 and 3.4.1 and entered code freeze on Friday. This means that 
we now only make small changes and mainly keep the release branches as is so 
they can be reviewed and tested for the release. We can make exceptions to this 
policy for very small changes or for pull requests that were already open on 
Friday and which we can close shortly.
Based on this, I think that our options here are:

1. Combine both pull requests, but merge them only after the 3.4.1 release 
since your PR is too big of a change to be merged during code freeze.
2. Merge this PR without bigger modifications and apply other changes after the 
3.4.1 release with follow-up PRs.

I tend towards option 2 here, because I would like to close TINKERPOP-2135 and 
TINKERPOP-2148 for this release and I don't think that we can make bigger 
changes to this PR and still merge it during code freeze.

In general, I think it would good if you could split your PR into multiple 
smaller ones that address ideally only one issue each. Some of the improvements 
you suggest are obvious improvements, but others will probably require some 
discussion. For example, [we decided 
earlier](https://github.com/apache/tinkerpop/pull/903#issuecomment-420292652) 
that we don't want to wait for a connection to become available with a timeout 
and instead fail fast by throwing an exception if no connection is available. 
I'm not saying that we have to stick with this decision if you have good 
arguments against it, but this will require some discussion.
If you [create issues in 
JIRA](https://issues.apache.org/jira/projects/TINKERPOP/issues) first for these 
issues that you want to fix/improve, then we can also discuss about them first 
in general before you put in the work to create a dedicated PR for the issue.
I think that it would be easier and more efficient to apply your improvements 
like this, but we could of course also review your PR at once if you prefer it 
that way. That will however most likely result in a big review with quite a few 
iterations until it's in a state where we can merge it.

@jorgebay: Do you agree?

[ Full content available at: https://github.com/apache/tinkerpop/pull/1077 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to