SvenRoederer commented on PR #2999: URL: https://github.com/apache/thrift/pull/2999#issuecomment-2236206862
> The idea was not to rewrite the whole thing. I initially started with a separate thrift-file for that case (https://github.com/apache/thrift/pull/2993/commits/f7cda292175673fef3c00052e99040a754719208, https://github.com/apache/thrift/pull/2993/commits/4f252f4a0a272d6f751aaa5f109acb1f385f842d), but realized that the Thrift-5794 also applies to the existing sourcefiles. For that reason I came up with that "large change" version. Idea behind it was to use default compiler option in favor of specific ones. And use explicit options only where needed (like the net8/C#12 tests-folder). I'm aware, that this solution adds more files that need to be maintained, slightly increases complexity but also gives some better test-coverage. As both version for the test are around for review and @Jens-G prefers the "separate tests-file" version for the project, I'm fine. For that reason I made small commits to allow cherry-picking. I hope you was able to make use of it for PR #3000. > The net8 option is very deliberately placed there. Is that the reason, which causes the "Test_Complex_DeepCopy" to fail with non net8/net6? > I occasionally think about making the last release the default (currently net8) ... C#12 by default would break our (legacy) project and we would need to have a "legacy" option when compiling. But even with net8 by default and other levels as option, a test will be needed, which goes back to above "separate tests-files vs. same tests for all language-levels". -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@thrift.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
