On Fri, Oct 15, 2010 at 12:11 PM, Henry Robinson <he...@cloudera.com> wrote:
> The netty patch is a good test case for this approach. If we feel that > reworking the structure of the existing server cnxn code will make it > significantly easier to add a second implementation that adheres to the > same > interface, then I say that such a refactoring is worthwhile, but even then > only if it's straightforward to make the changes while convincing ourselves > that the behaviour of the new implementation is consistent with the old. > > Thomas, do comment on the patch itself! That's the very best way to make > sure your concerns get heard and addressed. > Well really the _best_ way IMO is to both comment and submit a patch. ;-) And this is just what Thomas is doing, so kudos to him for the effort! Vishal is doing this as well for many of the issues he's found, so thanks to him also. We do appreciate you guys jumping in to help. Lack of contributors is one of the things we've been missing and addressing that opens the door to some of these improvements being suggested. Wrt the netty patch, the approach Ben and I took was to refactor sufficiently to add support for NIO/Netty/... while minimizing breakage. This is already a big patch, esp given that the code is not really as clean to begin with (complex too). Perfect situation, no. But the intent was to further clean things up once the original patch was reviewed/committed. Trying to do a huge refactoring in one shot (one patch) is not a good idea imo. Already these patches are too large. Perhaps lesson learned here is that we should have just created a special branch from the get go, applied a number of smaller patches to that branch, then eventually merged back into the trunk once it was fully baked. Patrick
