[GitHub] [arrow] pitrou commented on a change in pull request #11969: ARROW-15121: [C++] Implement max recursion on GcsFileSystem

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

pitrou commented on a change in pull request #11969:
URL: https://github.com/apache/arrow/pull/11969#discussion_r772306994



##########
File path: cpp/src/arrow/filesystem/gcsfs_internal.cc
##########
@@ -258,6 +259,10 @@ Result<std::shared_ptr<const KeyValueMetadata>> FromObjectMetadata(
   return result;
 }
 
+std::size_t Depth(arrow::util::string_view path) {

Review comment:
       Nit, but as per the [coding guidelines](https://arrow.apache.org/docs/developers/cpp/development.html#code-style-linting-and-ci) (which are copied over from the Google C++ style guide :-)), we generally use explicit-width types such as `int32_t`.

##########
File path: cpp/src/arrow/filesystem/gcsfs_test.cc
##########
@@ -591,6 +634,7 @@ TEST_F(GcsIntegrationTest, DeleteDirSuccess) {
   arrow::fs::AssertFileInfo(fs.get(), PreexistingBucketPath(), FileType::Directory);
   arrow::fs::AssertFileInfo(fs.get(), PreexistingObjectPath(), FileType::File);
   for (auto const& info : hierarchy.contents) {
+    if (!fs::internal::IsAncestorOf(hierarchy.base_dir, info.path())) continue;

Review comment:
       You could actually check that this entry still exists instead of ignoring it.
   (same comment below too)




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