address PR comments

rerun-io · Nov 30, 2023 · c4688be · c4688be
1 parent 54f92d9
commit c4688be
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 11 deletions.
diff --git a/examples/cpp/shared_recording/main.cpp b/examples/cpp/shared_recording/main.cpp
@@ -18,11 +18,7 @@ int main() {
         rerun::RecordingStream("rerun_example_shared_recording", "my_shared_recording");
     rec.spawn().exit_on_failure();
 
-    int pid = getpid();
-    std::ostringstream oss;
-    oss << "Hello from " << pid;
-
-    rec.log("updates", rerun::TextLog(oss.str()));
+    rec.log("updates", rerun::TextLog(std::string("Hello from ") + std::to_string(getpid())));
 
     std::cout << "Run me again to append more data to the recording!" << std::endl;
 }
diff --git a/rerun_cpp/src/rerun/recording_stream.cpp b/rerun_cpp/src/rerun/recording_stream.cpp
@@ -30,7 +30,7 @@ namespace rerun {
     }
 
     RecordingStream::RecordingStream(
-        std::string_view app_id, std::optional<std::string_view> recording_id, StoreKind store_kind
+        std::string_view app_id, std::string_view recording_id, StoreKind store_kind
     )
         : _store_kind(store_kind) {
         check_binary_and_header_version_match().handle();

diff --git a/rerun_cpp/src/rerun/recording_stream.hpp b/rerun_cpp/src/rerun/recording_stream.hpp
@@ -62,7 +62,7 @@ namespace rerun {
         /// \param recording_id The user-chosen name of the recording being logged to.
         /// \param store_kind Whether to log to the recording store or the blueprint store.
         RecordingStream(
-            std::string_view app_id, std::optional<std::string_view> recording_id = std::nullopt,
+            std::string_view app_id, std::string_view recording_id = std::string_view(),
             StoreKind store_kind = StoreKind::Recording
         );
         ~RecordingStream();

diff --git a/rerun_cpp/tests/recording_stream.cpp b/rerun_cpp/tests/recording_stream.cpp
@@ -74,7 +74,7 @@ SCENARIO("RecordingStream can be created, destroyed and lists correct properties
         AND_GIVEN("a valid application id") {
             THEN("creating a new stream does not log an error") {
                 rerun::RecordingStream stream = check_logged_error([&] {
-                    return rerun::RecordingStream("rerun_example_test", std::nullopt, kind);
+                    return rerun::RecordingStream("rerun_example_test", std::string_view(), kind);
                 });
 
                 AND_THEN("it does not crash on destruction") {}
@@ -100,7 +100,7 @@ SCENARIO("RecordingStream can be created, destroyed and lists correct properties
         AND_GIVEN("invalid utf8 character sequence for the application id") {
             THEN("creating a new stream logs an invalid string argument error") {
                 check_logged_error(
-                    [&] { rerun::RecordingStream stream("\xc3\x28", std::nullopt, kind); },
+                    [&] { rerun::RecordingStream stream("\xc3\x28", std::string_view(), kind); },
                     rerun::ErrorCode::InvalidStringArgument
                 );
             }
@@ -124,7 +124,7 @@ SCENARIO("RecordingStream can be set as global and thread local", TEST_TAG) {
             }
 
             WHEN("creating a new stream") {
-                rerun::RecordingStream stream("test", std::nullopt, kind);
+                rerun::RecordingStream stream("test", std::string_view(), kind);
 
                 THEN("it can be set as global") {
                     stream.set_global();
@@ -147,7 +147,7 @@ SCENARIO("RecordingStream can be used for logging archetypes and components", TE
     for (auto kind : std::array{rerun::StoreKind::Recording, rerun::StoreKind::Blueprint}) {
         GIVEN("a store kind" << kind) {
             WHEN("creating a new stream") {
-                rerun::RecordingStream stream("test", std::nullopt, kind);
+                rerun::RecordingStream stream("test", std::string_view(), kind);
 
                 // We can make single components work, but this would make error messages a lot
                 // worse since we'd have to implement the base `AsComponents` template for this.