You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/17 02:30:09 UTC

[GitHub] [pulsar] Demogorgon314 opened a new pull request #13369: [C++] Adjust clang-format search names

Demogorgon314 opened a new pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369


   ### Motivation
   
   The Pulsar C++ client use `clang-format` 5.0 to format files, we don't need to search for a version lower than 5.0.
   
   Additionally, the old version of clang-format might be hard to install in the normal way, for example, in macOS,  users can't use brew to install clang-format@5, because brew doesn't maintain the old version software with very few users. In this PR, it introduced a solution to install `clang-format@5` in macOS.
   
   ### Modifications
   
   * Make CMake don't search clang-format for a version lower than 5.0.
   * Introduce solution to install clang-format 5.0
   
   ### Documentation
   
   Need to update docs? 
   
   - [x] `doc` 
     
   
   
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Anonymitaet merged pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Anonymitaet merged pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369


   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Anonymitaet commented on a change in pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369#discussion_r772045996



##########
File path: pulsar-client-cpp/README.md
##########
@@ -277,3 +277,23 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
 It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 5.0 to format files, which is a little different with latest `clang-format`.
 
 We welcome contributions from the open source community, kindly make sure your changes are backward compatible with GCC 4.8 and Boost 1.53.
+
+### Install `clang-format` on macOS
+
+Since the homebrew-core doesn't have `clang-format@5`, here is a tap to install `clang-format@5` in your macOS.

Review comment:
       ```suggestion
   `homebrew-core` does not have `clang-format@5`. You can install `clang-format@5` on your macOS using the tap below.
   ```
   
   1. Do you mean this?
   
   
   2. Do not use contractions in technical information. Reason: https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit#bookmark=id.qa5rwiehb245

##########
File path: pulsar-client-cpp/cmake_modules/FindClangTools.cmake
##########
@@ -82,11 +82,8 @@ if (CLANG_FORMAT_VERSION)
     endif()
 else()
     find_program(CLANG_FORMAT_BIN
-            NAMES clang-format-4.0
-            clang-format-3.9
-            clang-format-3.8
-            clang-format-3.7
-            clang-format-3.6
+            NAMES clang-format-5

Review comment:
       @BewareMyPower can you review this PR from the technical perspective? Thanks




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Demogorgon314 commented on pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369#issuecomment-996404693


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Demogorgon314 commented on pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369#issuecomment-996612565


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Demogorgon314 commented on pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369#issuecomment-996443941


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Demogorgon314 commented on a change in pull request #13369: [C++] Adjust clang-format search names

Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #13369:
URL: https://github.com/apache/pulsar/pull/13369#discussion_r772050613



##########
File path: pulsar-client-cpp/README.md
##########
@@ -277,3 +277,23 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
 It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 5.0 to format files, which is a little different with latest `clang-format`.
 
 We welcome contributions from the open source community, kindly make sure your changes are backward compatible with GCC 4.8 and Boost 1.53.
+
+### Install `clang-format` on macOS
+
+Since the homebrew-core doesn't have `clang-format@5`, here is a tap to install `clang-format@5` in your macOS.

Review comment:
       Thanks for your suggestion. Fixed.




-- 
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: commits-unsubscribe@pulsar.apache.org

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