You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/10/13 13:23:55 UTC

[GitHub] [arrow] bkmgit opened a new pull request #11399: ARROW-14291: [C++] Use CI for linting

bkmgit opened a new pull request #11399:
URL: https://github.com/apache/arrow/pull/11399


   lint all cc files including examples


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



[GitHub] [arrow] github-actions[bot] commented on pull request #11399: ARROW-14291: [C++] Use CI for linting

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-942304452


   https://issues.apache.org/jira/browse/ARROW-14291


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



[GitHub] [arrow] kou commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730574802



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       I tried `static std::string` in function not class and the following error could be suppressed:
   
   https://github.com/apache/arrow/runs/3883052477#step:7:5583
   
   ```text
   /arrow/cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc:171:  Static/global string variables are not permitted.  [runtime/string] [4]
   ```
   
   Is the `static std::string` in function denied in our lint?




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



[GitHub] [arrow] kou commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730575870



##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

Review comment:
       This can be out-of-scope of this pull request.




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



[GitHub] [arrow] kou commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730497854



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       How about this?
   
   ```c++
   static std::string string;
   string = "StringView #" + std::to_string(i);
   return arrow::util::string_view(string);
   ```

##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

Review comment:
       Should we include this change into this pull request?




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



[GitHub] [arrow] bkmgit commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957920093






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



[GitHub] [arrow] ursabot edited a comment on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-946366091


   Benchmark runs are scheduled for baseline = d9ef519f458fc5989fd15f0af49f069c34110c35 and contender = b3767f78aacf7537b967f6c2f2f89cface3f6952. b3767f78aacf7537b967f6c2f2f89cface3f6952 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fc83bba11fcd43f08b0eb86a45d29bf9...dd4fa1b500544742a5cb68dcc4baf543/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/384eee012ee04f74a64e385b10a8810b...3d97fa32c4fe46d9b136f3fc1759c422/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44e3cedae529460ab28d50c1a11b9a07...20b0d93bf54a4d449dd8431c0c2821d4/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


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



[GitHub] [arrow] github-actions[bot] commented on pull request #11399: ARROW-14291: [C++] Use CI for linting

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-942304452


   https://issues.apache.org/jira/browse/ARROW-14291


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



[GitHub] [arrow] pitrou commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957909335


   `archery lint` is now linting unrelated source files in build directories inside `cpp`.


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



[GitHub] [arrow] kou commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730575594



##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

Review comment:
       We can use ARROW-14316 for this work if we need this work.




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



[GitHub] [arrow] kou closed pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou closed pull request #11399:
URL: https://github.com/apache/arrow/pull/11399


   


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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730567907



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       ```cpp
   static std::string string
   ```
   is not recommended style. [Style guide|https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables] suggests using a C character array
   ```cpp
       static char string[100];
       int length = snprintf(string, str_buff_size, "StringView #%d", i);
      snprintf(string, length + 1, "StringView #%d", i);
      return arrow::util::string_view(string);
   ```
   otherwise would need to make an exception in linting.




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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730586129



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       Thanks. That makes them local and it is easier to read and maintain.




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



[GitHub] [arrow] bkmgit commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957920093


   Ok. Can update it. Is there a particular run to examine? Can specify the directories to lint more precisely.


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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730567907



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       ```cpp
   static std::string string
   ```
   is not recommended style. [Style guide](https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables) suggests using a C character array
   ```cpp
       static char string[100];
       int length = snprintf(string, str_buff_size, "StringView #%d", i);
      snprintf(string, length + 1, "StringView #%d", i);
      return arrow::util::string_view(string);
   ```
   otherwise would need to make an exception in linting.




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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730579546



##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

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

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



[GitHub] [arrow] bkmgit commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957928413


   @pitrou Ok. [Lint exclusions file](https://github.com/apache/arrow/blob/master/cpp/build-support/lint_exclusions.txt) was updated, but the build directory can have any name. The examples were not being linted, and it would be good to include these in addition to the source code - this is what motivated the change.


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



[GitHub] [arrow] ursabot commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-946366091


   Benchmark runs are scheduled for baseline = d9ef519f458fc5989fd15f0af49f069c34110c35 and contender = b3767f78aacf7537b967f6c2f2f89cface3f6952. b3767f78aacf7537b967f6c2f2f89cface3f6952 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fc83bba11fcd43f08b0eb86a45d29bf9...dd4fa1b500544742a5cb68dcc4baf543/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/384eee012ee04f74a64e385b10a8810b...3d97fa32c4fe46d9b136f3fc1759c422/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44e3cedae529460ab28d50c1a11b9a07...20b0d93bf54a4d449dd8431c0c2821d4/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730570574



##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

Review comment:
       Will make running tests on wider range of docker-compose installations easier.




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



[GitHub] [arrow] kou commented on pull request #11399: ARROW-14291: [C++] Use CI for linting

Posted by GitBox <gi...@apache.org>.
kou commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-942646949


   We need to fix format of some new target files. e.g.: https://github.com/apache/arrow/pull/11399/checks?check_run_id=3883052477#step:7:783
   
   We need to add more patterns to `cpp/build-support/lint_exclusions.txt`. For example, we don't want to format thirdparty files: https://github.com/apache/arrow/pull/11399/checks?check_run_id=3883052477#step:7:810


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



[GitHub] [arrow] ursabot edited a comment on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-946366091


   Benchmark runs are scheduled for baseline = d9ef519f458fc5989fd15f0af49f069c34110c35 and contender = b3767f78aacf7537b967f6c2f2f89cface3f6952. b3767f78aacf7537b967f6c2f2f89cface3f6952 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fc83bba11fcd43f08b0eb86a45d29bf9...dd4fa1b500544742a5cb68dcc4baf543/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/384eee012ee04f74a64e385b10a8810b...3d97fa32c4fe46d9b136f3fc1759c422/)
   [Finished :arrow_down:0.27% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44e3cedae529460ab28d50c1a11b9a07...20b0d93bf54a4d449dd8431c0c2821d4/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


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



[GitHub] [arrow] kou commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730575198



##########
File path: cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
##########
@@ -136,12 +137,14 @@ struct TestData {
     return "Str #" + std::to_string(i);
   }
   static arrow::util::string_view GetStringView(const int i) {
-    string_ = "StringView #" + std::to_string(i);
+    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
+    snprintf(string_, length + 1, "StringView #%d", i);
     return arrow::util::string_view(string_);

Review comment:
       I tried this:
   
   ```diff
   diff --git a/cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc b/cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
   index 474c9c286..464f210c7 100644
   --- a/cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
   +++ b/cpp/examples/parquet/parquet_stream_api/stream_reader_writer.cc
   @@ -137,14 +137,14 @@ struct TestData {
        return "Str #" + std::to_string(i);
      }
      static arrow::util::string_view GetStringView(const int i) {
   -    int length = snprintf(string_, str_buff_size, "StringView #%d", i);
   -    snprintf(string_, length + 1, "StringView #%d", i);
   -    return arrow::util::string_view(string_);
   +    static std::string string;
   +    string = "StringView #" + std::to_string(i);
   +    return arrow::util::string_view(string);
      }
      static const char* GetCharPtr(const int i) {
   -    int length = snprintf(string_, str_buff_size, "CharPtr #%d", i);
   -    snprintf(string_, length + 1, "CharPtr #%d", i);
   -    return string_;
   +    static std::string string;
   +    string = "CharPtr #" + std::to_string(i);
   +    return string.c_str();
      }
      static char GetChar(const int i) { return i & 1 ? 'M' : 'F'; }
      static int8_t GetInt8(const int i) { return static_cast<int8_t>((i % 256) - 128); }
   @@ -166,7 +166,6 @@ struct TestData {
    
     private:
      static std::time_t ts_offset_;
   -  static char string_[str_buff_size];
    };
    
    char TestData::char4_array[] = "XYZ";
   ```




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



[GitHub] [arrow] pitrou commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957926545


   @bkmgit I'm working on a fix already.


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



[GitHub] [arrow] bkmgit commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957920093






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



[GitHub] [arrow] ursabot edited a comment on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-946366091


   Benchmark runs are scheduled for baseline = d9ef519f458fc5989fd15f0af49f069c34110c35 and contender = b3767f78aacf7537b967f6c2f2f89cface3f6952. b3767f78aacf7537b967f6c2f2f89cface3f6952 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fc83bba11fcd43f08b0eb86a45d29bf9...dd4fa1b500544742a5cb68dcc4baf543/)
   [Finished :arrow_down:1.03% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/384eee012ee04f74a64e385b10a8810b...3d97fa32c4fe46d9b136f3fc1759c422/)
   [Finished :arrow_down:0.27% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44e3cedae529460ab28d50c1a11b9a07...20b0d93bf54a4d449dd8431c0c2821d4/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


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



[GitHub] [arrow] pitrou commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957909335






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



[GitHub] [arrow] pitrou commented on pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#issuecomment-957909335






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



[GitHub] [arrow] bkmgit commented on a change in pull request #11399: ARROW-14291: [CI][C++] Add cpp/examples/ files to lint targets

Posted by GitBox <gi...@apache.org>.
bkmgit commented on a change in pull request #11399:
URL: https://github.com/apache/arrow/pull/11399#discussion_r730568330



##########
File path: docker-compose.yml
##########
@@ -1065,9 +1065,26 @@ services:
         /arrow/ci/scripts/r_test.sh /arrow"
 
   ubuntu-r-only-r:
+    image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+    build:
+      context: .
+      dockerfile: ci/docker/linux-apt-r.dockerfile
+      cache_from:
+        - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}
+      args:
+        arch: ${ARCH}
+        r: ${R}
+        base: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp
+        gcc_version: ${GCC_VERSION}
+        tz: ${TZ}
+    shm_size: *shm-size
     environment:
-      ARROW_DEPENDENCY_SOURCE: ''
-    extends: ubuntu-r
+      <<: *ccache
+      ARROW_R_CXXFLAGS: '-Werror'
+      LIBARROW_BUILD: 'false'
+      NOT_CRAN: 'true'
+      ARROW_R_DEV: ${ARROW_R_DEV}
+    volumes: *ubuntu-volumes

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

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