I did not submit this patch but I have been following the patch process for a 
while as I am interested in this client. So I have some comments also:

> •     Strong naming keys or dependent dll’s aren’t part of the patch but I 
> think that’s likely due to the fact that patches can’t contain binaries but 
> it does mean I had to source the dll’s from somewhere and disable strong 
> naming for now.

The strong naming key should not be submitted or obtainable from source 
control. An extra step should be generated in a NAnt build file to sign the 
Assembly with a SNK on the build machine. This SNK should be held exclusively 
by Apache (ZooKeeper). 

> •     Tests require a locally running ZooKeeper server which is annoying and 
> means there’s currently no way of running the tests automatically as the 
> build.xml does nothing for .Net other than running jute.

I have a running build of ZooKeeper using IKVM that can be used to embed a 
QuorumPeer into unit testing. This process can be included in an updated NAnt 
build file in order to successfully test the client. However, this would take 
the C# client off the same process the other clients go through. The other 
clients must have a way to start ZK servers, correct?


The Assembly needs to be named Apache.ZooKeeper.Client as ZooKeeperNet does not 
adhere to any .NET naming standard. 

I can fix most of these or work with Eric in doing so if needed. There isn't a 
mirror I can pull off for these as far as I can tell correct? It is a temporary 
build that applies the patch then runs through the tests? If not I'll fork from 
Eric's build.

Andrew

On Sep 6, 2011, at 10:28 AM, Camille Fournier (JIRA) wrote:

> 
>    [ 
> https://issues.apache.org/jira/browse/ZOOKEEPER-1158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098030#comment-13098030
>  ] 
> 
> Camille Fournier commented on ZOOKEEPER-1158:
> ---------------------------------------------
> 
> Some comments from our side of the fence on the first patch, please disregard 
> those that are not relevant with the updates:
> 
> •     After applying the patch, a lot of the files seem to have weird 
> characters (maybe need a dos2unix?).  I’ve seen them in svn diffs before so 
> thought it may be normal but doesn’t seem like it.  Had to manually remove 
> them.
> •     The csproj doesn’t fully correspond to the jute generated files; 
> there’s an extra file required by the csproj which jute doesn’t generate.  
> Luckily it’s not required by the code, just a bad csproj.
> •     Mismatch between code and jute templates (jute generates 
> ZooKeeperNet.Conversion but real namespace is ZooKeeperNet.IO).
> •     Strong naming keys or dependent dll’s aren’t part of the patch but I 
> think that’s likely due to the fact that patches can’t contain binaries but 
> it does mean I had to source the dll’s from somewhere and disable strong 
> naming for now.
> •     One of the csproj’s has a reference to pnunit.  I’ve no idea what it is 
> but it doesn’t seem required.  Again, bad csproj.
> •     Some weird nunit stuff, he uses Assert.True but I’ve only ever seen 
> (and the only version my nunit has) is Assert.IsTrue.
> •     Tests require a locally running ZooKeeper server which is annoying and 
> means there’s currently no way of running the tests automatically as the 
> build.xml does nothing for .Net other than running jute.
> 
> 
> We did manage to work through these and get the unit tests to pass, so I 
> think it's all quite promising. We also have a few small enhancements that we 
> made over the old SharpKeeper that don't seem to exist here either but we can 
> add those in once this is committed.
> 
>> C# client
>> ---------
>> 
>>                Key: ZOOKEEPER-1158
>>                URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1158
>>            Project: ZooKeeper
>>         Issue Type: Improvement
>>           Reporter: Eric Hauser
>>           Assignee: Eric Hauser
>>            Fix For: 3.5.0
>> 
>>        Attachments: ZOOKEEPER-1158-09032011.patch
>> 
>> 
>> Native C# client for ZooKeeper.
> 
> --
> This message is automatically generated by JIRA.
> For more information on JIRA, see: http://www.atlassian.com/software/jira
> 
> 

Reply via email to