[GitHub] [thrift] fishy commented on a diff in pull request #2783: THRIFT-5693 - fix bug in serialization of enum default values

["fishy (via GitHub)" <gi...@apache.org>]

fishy commented on code in PR #2783:
URL: https://github.com/apache/thrift/pull/2783#discussion_r1167099967


##########
compiler/cpp/src/thrift/generate/t_py_generator.cc:
##########
@@ -2662,6 +2662,11 @@ string t_py_generator::declare_argument(t_field* tfield) {
 string t_py_generator::render_field_default_value(t_field* tfield) {
   t_type* type = get_true_type(tfield->get_type());
   if (tfield->get_value() != nullptr) {
+    if (type->is_enum() && gen_enum_) {
+        std::ostringstream out;
+        out << tfield->get_value()->get_integer();
+        return out.str();
+        }

Review Comment:
   I see this diff from `ttypes.py`:
   ```diff
   @@ -2931,7 +2931,7 @@ class OptionalEnum(object):
        )
    
    
   -    def __init__(self, e=    Numberz.FIVE,):
   +    def __init__(self, e=5,):
            self.e = e
    
        def read(self, iprot):
   ```
   and also this diff:
   ```diff
   @@ -7198,7 +7198,7 @@ OptionalBinary.thrift_spec = (
    all_structs.append(OptionalEnum)
    OptionalEnum.thrift_spec = (
        None,  # 0
   -    (1, TType.I32, 'e', None,     Numberz.FIVE, ),  # 1
   +    (1, TType.I32, 'e', None, 5, ),  # 1
    )
    fix_spec(all_structs)
    del all_structs
   ```
   which I think is caused by this part of the change. from the diff I don't think this change is necessary? (it's probably still correct, but the old code generated seems to be better?)



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