[GitHub] [arrow] lidavidm commented on a diff in pull request #13383: ARROW-16769: [C++] Add Warn() function to Status

[GitBox <gi...@apache.org>]

lidavidm commented on code in PR #13383:
URL: https://github.com/apache/arrow/pull/13383#discussion_r897870038


##########
cpp/src/arrow/status.cc:
##########
@@ -131,6 +131,17 @@ void Status::Abort(const std::string& message) const {
   std::abort();
 }
 
+void Status::Warn() const { Warn(std::string()); }
+
+void Status::Warn(const std::string& message) const {
+  ARROW_LOG(WARNING) << "-- Arrow Warning --";
+  if (!message.empty()) {
+    ARROW_LOG(WARNING) << message;
+  }
+
+  ARROW_LOG(WARNING) << ToString() << "\n";

Review Comment:
   FWIW, it might be better to consolidate the message and status into a single log so that concurrent logs don't get mixed up.
   
   ```suggestion
     ARROW_LOG(WARNING) << "-- Arrow Warning --";
     if (!message.empty()) {
       ARROW_LOG(WARNING) << message;
     }
   
     ARROW_LOG(WARNING) << ToString();
   ```



##########
cpp/src/arrow/status.cc:
##########
@@ -131,6 +131,17 @@ void Status::Abort(const std::string& message) const {
   std::abort();
 }
 
+void Status::Warn() const { Warn(std::string()); }
+
+void Status::Warn(const std::string& message) const {
+  ARROW_LOG(WARNING) << "-- Arrow Warning --";
+  if (!message.empty()) {
+    ARROW_LOG(WARNING) << message;
+  }
+
+  ARROW_LOG(WARNING) << ToString() << "\n";

Review Comment:
   That way even the original status messages will keep working



##########
cpp/src/arrow/status.cc:
##########
@@ -131,6 +131,17 @@ void Status::Abort(const std::string& message) const {
   std::abort();
 }
 
+void Status::Warn() const { Warn(std::string()); }
+
+void Status::Warn(const std::string& message) const {
+  ARROW_LOG(WARNING) << "-- Arrow Warning --";
+  if (!message.empty()) {
+    ARROW_LOG(WARNING) << message;
+  }
+
+  ARROW_LOG(WARNING) << ToString() << "\n";

Review Comment:
   I would honestly just make it
   
   ```suggestion
     ARROW_LOG(WARNING) << "-- Arrow Warning --";
     ARROW_LOG(WARNING) << message << ToString();
   ```



-- 
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: github-unsubscribe@arrow.apache.org

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