[
https://issues.apache.org/jira/browse/THRIFT-3397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14977200#comment-14977200
]
Jens Geyer commented on THRIFT-3397:
------------------------------------
As Randy pointed out, nobody really likes monster patches. Your patch surely
falls not into that category, so no danger on that end.
{quote}
I have tried to ensure that each commit is limited to a particular change to
aide traceability, however it looks like most of the existing history contains
a single commit per JIRA ticket.
{quote}
That's right, the PRs are squashed when we merge them (ideally they are
squashed before). So in essence, the granularity of the patches is based on
JIRA tickets. The only situation where monster patches are (sort of) acceptable
is when integrating a new language with compiler, library, test and tutorial -
although the latter two coold also be separate patches.
{quote}
What granularity is preferred for change control? e.g. Should I be logging
separate JIRA tickets for Library code vs Test code?
{quote}
Each should address one set of changes that not only belong together, but also
make technical sense. Ergo splitting one logical change set into two
patches/PRs for compiler and library is not really a good idea, unless they
address entirely different issues. The other end of the continuum is creating 5
tickets for just one logical group of minorish patches, each of them addressing
one particular sub-aspect of the greater picture. This approach produces
unnecessary noise on the mailing lists without providing any real value, and
therefore should be avoided as well.
A good rule of thumb is to put on the (mental) hat of a reviewer and ask
yourself, whether you would like to review your own PR with regard to it's size
and the problems covered, or not.
> Implement TProcessorFactory in C# to enable per-client processors
> -----------------------------------------------------------------
>
> Key: THRIFT-3397
> URL: https://issues.apache.org/jira/browse/THRIFT-3397
> Project: Thrift
> Issue Type: New Feature
> Components: C# - Library
> Affects Versions: 0.9.3
> Reporter: Jonathan Heard
> Priority: Minor
> Labels: performance, thread-safety
> Original Estimate: 168h
> Remaining Estimate: 168h
>
> The current TServer implementations in C# take a single instance of a
> TProcessor to be used for all client connections. This is not guaranteed to
> be thread-safe and can become a significant bottleneck.
> I am implementing a TProcessorFactory interface for C# which is similar to
> the ones already implemented in Java and C++. I'll generate a pull request
> for review soon.
> The existing solutions implement TProcessorFactory as a class which takes a
> TProcessor and just returns that instance to all clients. I'm aiming to
> improve on that by creating a TProcessorFactory interface and then
> implementing it in two core classes:
> 1) TSingletonProcessorFactory - this behaves the same as the Java
> 'TProcessorFactory' class.
> 2) TPrototypeProcessorFactory<Processor,Handler> - this effectively returns a
> 'new Processor(Handler)' giving every new client its own processor.
> In order to avoid breaking the existing API (and in-keeping with the Java
> implementation), I have changed TServer so that it uses a TProcessorFactory
> instead of TProcessor and updated all relevant constructors so they call the
> TSingletonProcessorFactory for constructors using TProcessor. New
> constructors have been added to enable calling with TProcessorFactory. This
> approach should avoid breaking the established API and not break any existing
> code.
> I've also updated the TestServer.cs to support these changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
