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 [email protected] or file a JIRA ticket
with INFRA.
---
