empiredan commented on code in PR #1933:
URL:
https://github.com/apache/incubator-pegasus/pull/1933#discussion_r1515882709
##########
src/replica/bulk_load/test/replica_bulk_loader_test.cpp:
##########
@@ -404,7 +405,6 @@ class replica_bulk_loader_test : public replica_test_base
file_meta _file_meta;
bulk_load_metadata _metadata;
- std::string APP_NAME = "replica";
Review Comment:
I think this `APP_NAME` is the name of a table rather than the service app
(meta/replica, i.e. `replication_options::kReplicaAppType`). To clear this, we
could use some name like `"replica_bulk_loader_test"` as the table name instead
of `"replica"`.
##########
src/replica/replication_app_base.cpp:
##########
@@ -119,6 +120,9 @@ error_code replica_app_info::store(const std::string &fname)
fname, writer.get_buffer(), dsn::utils::FileDataType::kSensitive);
}
+const std::string replication_app_base::kDataDirPostfix = "data";
+const std::string replication_app_base::kRdbPostfix = "rdb";
Review Comment:
Better to name them as `kDataDir` and `kRdbDir` ?
##########
src/replica/replication_app_base.h:
##########
@@ -123,6 +126,8 @@ class replication_app_base : public replica_base
typedef replication_app_base *factory(replica *r);
static void register_storage_engine(const std::string &name, factory f);
static replication_app_base *new_storage_instance(const std::string &name,
replica *r);
+ static const std::string kDataDirPostfix;
+ static const std::string kRdbPostfix;
Review Comment:
Need to also replace `"data"` and `"rdb"` in other files such as
`src/server/pegasus_server_impl.cpp`
`src/server/test/pegasus_server_test_base.h` ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
