Github user Jens-G commented on the issue:

    https://github.com/apache/thrift/pull/1088
  
    ### General
    
    All code files etc. need an ASF header on top. Please have a look at the 
existing code if you are unsure, what, where and how to apply.
    
    ### using Serilog;  
    
    We don't like external dependencies that much. Thrift supports about 20+ 
languages and dialects, each additional dependency has the potential to quickly 
become a PITA. So we avoid them wherever possible.
    
    ### lib/netcore/src/Samples
    
    If that is the Thrift Test Suite client/server pair, that part should be 
move to /test/netcore. All other tests should remain under /lib/netcore. In 
addition to that, we have a [standardized set of arguments and exit 
codes](https://thrift.apache.org/test/). I haven't checked how close these 
program(s) already adhere to them, but from a first quick glance it may not 
100%. 
    
    ### Compatibility I
    
    Have you tested the code against at least one other language, preferably 
C++, Java or C#?
    
    ### Compatibility II
    
    Can at least the Thrift Test Suite .NET Core Application  code be built on 
Linux and/or Mac? 
    
    ### AssemblyInfo.cs
    
    Should be set up according to the existing C# code.
    
    ### /lib/netcore/src/Samples/.../Calculator.cs
    
    The Tutorial code belongs under /tutorial/netcore. The code is later to be 
[included on the web site](https://thrift.apache.org/tutorial/) automagically 
from that folder. 
    
    Next, generated code should generally not be put into the source tree. Part 
of the tutorial is to create these files from the IDL file.
    
    ### Summary
    
    Great work, looks promising! But as I said, only a first look, however 
still plenty of room for improvement ;-) Have you seen [that 
document](https://thrift.apache.org/docs/HowToNewLanguage)?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to