Github user bufferoverflow commented on the pull request:

    https://github.com/apache/thrift/pull/731#issuecomment-162352720
  
    Great patch Andrew!
    I would really like to get FLOAT up and running here. The blocker for not 
merging now is breaking other languages with this at the moment, I had to 
exclude a lot:
    ```
    sh bootstrap.sh
    ./configure --without-go --without-c_glib --without-haskell --without-haxe 
--without-java --without-erlang --without-ruby --without-perl --without-php 
--without-nodejs --without-python --without-csharp
    make
    make check
    make cross
    python test/test.py -s --server cpp --client cpp
    ```
    our first line of defense was hit heavily ;-) 
https://travis-ci.org/apache/thrift/builds/94772477
    
    Could you fix these issues?
    ```
    In member function ‘std::string 
t_csharp_generator::declare_field(t_field*, bool, std::string)’:
    src/generate/t_csharp_generator.cc:2713:14: warning: enumeration value 
‘TYPE_FLOAT’ not handled in switch [-Wswitch]
           switch (tbase) {
    ```
    I would say providing a warning such as here  and add some little stubs 
with a TODO should be sufficient:
    
https://github.com/apache/thrift/blob/master/compiler/cpp/src/generate/t_as3_generator.cc#L2358
    
    thanks again!
    -roger



---
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