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/03/05 08:01:55 UTC

[GitHub] [pulsar] zymap opened a new pull request #9816: Fix the apache-rat check

zymap opened a new pull request #9816:
URL: https://github.com/apache/pulsar/pull/9816


   ---
   
   **Motivation**
   
   PR #9072 introduces the gtest file but does not exclude for apache-rat
   check. So that causes the apache-rat check to fail.
   


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

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

[GitHub] [pulsar] BewareMyPower commented on a change in pull request #9816: Fix the apache-rat check

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



##########
File path: pulsar-client-cpp/lib/TestUtil.h
##########
@@ -0,0 +1,10 @@
+//
+// Created by yong on 2021/3/5.
+//

Review comment:
       It should be replaced by the license header.




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

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

[GitHub] [pulsar] zymap commented on pull request #9816: Fix the apache-rat check

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


   @merlimat @BewareMyPower Could you please take a look at this? 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.

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

[GitHub] [pulsar] BewareMyPower commented on pull request #9816: Fix the apache-rat check

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


   There're three places that included the `gtest_prod.h` before:
   
   ```
   lib/UnAckedMessageTrackerEnabled.h:21:#include "gtest/gtest_prod.h"
   lib/MultiTopicsConsumerImpl.h:21:#include "gtest/gtest_prod.h"
   lib/PartitionedConsumerImpl.h:21:#include "gtest/gtest_prod.h"
   ```
   
   You should replace it with `#include "TestUtils.h"`


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

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

[GitHub] [pulsar] merlimat merged pull request #9816: Remove gtest file from cpp library

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


   


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

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

[GitHub] [pulsar] zymap edited a comment on pull request #9816: Fix the apache-rat check

Posted by GitBox <gi...@apache.org>.
zymap edited a comment on pull request #9816:
URL: https://github.com/apache/pulsar/pull/9816#issuecomment-791285844


   @merlimat @BewareMyPower Could you please take a look at this? I remove the gtest file and move the macro into the lib dir. 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.

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

[GitHub] [pulsar] BewareMyPower edited a comment on pull request #9816: Fix the apache-rat check

Posted by GitBox <gi...@apache.org>.
BewareMyPower edited a comment on pull request #9816:
URL: https://github.com/apache/pulsar/pull/9816#issuecomment-791292836


   There're three places that included the `gtest_prod.h` before:
   
   ```
   lib/UnAckedMessageTrackerEnabled.h:21:#include "gtest/gtest_prod.h"
   lib/MultiTopicsConsumerImpl.h:21:#include "gtest/gtest_prod.h"
   lib/PartitionedConsumerImpl.h:21:#include "gtest/gtest_prod.h"
   ```
   
   You should replace it with `#include "TestUtil.h"`


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

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

[GitHub] [pulsar] merlimat commented on pull request #9816: Fix the apache-rat check

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


   A couple of notes on #9072: 
   
    1. That header file is just a tiny macro, we shouldn't need to import a file from gtest for that
    2. The macro is about tests and must not be exposed in public API
    


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

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

[GitHub] [pulsar] BewareMyPower commented on pull request #9816: Fix the apache-rat check

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


   Since the original `gtest_prod.h` has been removed, the pom.xml should remove following code as well.
   
   ```xml
    <exclude>pulsar-client-cpp/include/gtest/gtest_prod.h</exclude>
   ```


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

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

[GitHub] [pulsar] zymap commented on pull request #9816: Fix the apache-rat check

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


   We are not running the apache-rat so I added it at PR #9815


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

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

[GitHub] [pulsar] zymap commented on pull request #9816: Fix the apache-rat check

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


   @BewareMyPower I address your comments. Please take a look. thank you


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

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