You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/09/07 02:17:28 UTC

[GitHub] [thrift] yawaramin opened a new pull request, #2652: THRIFT-5208: capitalize exception ctor

yawaramin opened a new pull request, #2652:
URL: https://github.com/apache/thrift/pull/2652

   Fix https://issues.apache.org/jira/browse/THRIFT-5208


-- 
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: dev-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] Jens-G commented on pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2652:
URL: https://github.com/apache/thrift/pull/2652#issuecomment-1239795779

   Testcase would be nice. Possible?


-- 
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-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] yawaramin commented on pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
yawaramin commented on PR #2652:
URL: https://github.com/apache/thrift/pull/2652#issuecomment-1240173541

   OK for the record existing tests are building and running with the following setup:
   
   Configure test build:
   
   ```
   cd compiler/cpp/tests
   cmake -DCMAKE_PREFIX_PATH=/usr/local/opt/bison -DCMAKE_CXX_STANDARD=11 .
   ```
   
   Build and run tests:
   
   ```
   cmake --build .
   ctest
   ```
   
   _But,_ the existing netstd/netcore tests seem to have a naming mixup which is preventing any tests from _actually_ running, which I checked by deliberately introducing an incorrect input into the `t_netcore_generator_functional_tests_helpers.cc` file and seeing that no tests failed.
   
   I will try to write a test for the OCaml and see if I can actually get it to fail first before trying to add test coverage.


-- 
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-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] Jens-G commented on pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2652:
URL: https://github.com/apache/thrift/pull/2652#issuecomment-1243034222

   > But, the existing netstd/netcore tests seem to have a naming mixup which is preventing any tests from actually running, which I checked by deliberately introducing an incorrect input into the t_netcore_generator_functional_tests_helpers.cc file and seeing that no tests failed.
   
   Was not really aware about this piece of code. Never looked into it. Since it was written for netcore it might be broken anyway, there are a number of differences between the two implementations. If anybody wants to fix any issues here, please do. I most certainly won't do it myself.


-- 
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-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] yawaramin commented on pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
yawaramin commented on PR #2652:
URL: https://github.com/apache/thrift/pull/2652#issuecomment-1243064653

   I wasn't really asking for a fix nor volunteering, just keeping a record of my long and winding path to figuring out how to run the codegen unit tests :-)


-- 
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-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] yawaramin commented on pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
yawaramin commented on PR #2652:
URL: https://github.com/apache/thrift/pull/2652#issuecomment-1240099310

   @Jens-G any ideas where to start? I'm not a C++ guy so pretty unfamiliar with the tooling. Case in point, https://github.com/apache/thrift/blob/master/compiler/cpp/tests/README.md?plain=1#L52
   
   I'm not sure how to do that! E.g. how do I run specifically the netstd tests to see how it works?


-- 
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-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [thrift] Jens-G merged pull request #2652: THRIFT-5208: capitalize exception ctor

Posted by GitBox <gi...@apache.org>.
Jens-G merged PR #2652:
URL: https://github.com/apache/thrift/pull/2652


-- 
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: dev-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org