[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1491: refactor(new_metrics): refactor enum definition for metric types and units

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

acelyc111 commented on code in PR #1491:
URL: https://github.com/apache/incubator-pegasus/pull/1491#discussion_r1209448115


##########
src/utils/enum_helper.h:
##########
@@ -66,6 +66,11 @@
 
 #define ENUM_END(type) ENUM_END2(type, type)
 
+#define ENUM_CONST(str) k##str
+#define ENUM_CONST_DEF(str) ENUM_CONST(str),
+#define ENUM_CONST_REG_STR(enum_class, str)                                                        \
+    helper->register_enum(#str, enum_class::ENUM_CONST(str));

Review Comment:
   It would be great if adding a sample of how to use the serial of macros, and the related enum helpers, e.g. convert from/to std::string.



##########
src/replica/test/replica_disk_test_base.h:
##########
@@ -57,7 +57,6 @@ class replica_disk_test_base : public replica_test_base
         generate_mock_app_info();
 
         stub->_fs_manager._dir_nodes.clear();
-        stub->_fs_manager.reset_disk_stat();

Review Comment:
   Is it related to this patch?



##########
src/utils/enum_helper.h:
##########
@@ -66,6 +66,11 @@
 
 #define ENUM_END(type) ENUM_END2(type, type)
 
+#define ENUM_CONST(str) k##str
+#define ENUM_CONST_DEF(str) ENUM_CONST(str),
+#define ENUM_CONST_REG_STR(enum_class, str)                                                        \
+    helper->register_enum(#str, enum_class::ENUM_CONST(str));

Review Comment:
   Could you please add some tests to check the enum_helper_xxx works well as expect? Use the existing `metric_type` is OK.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org