You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/02/02 11:04:48 UTC

[GitHub] [skywalking] JohnNiang opened a new pull request #4309: Move lombok dependency into root dependency manager for easier version management

JohnNiang opened a new pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309
 
 
   Please answer these questions before submitting pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [x] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   https://github.com/apache/skywalking/issues/4284
   
   ___
   ### New feature or improvement
   
   Move lombok dependency into root dependency manager for easier version management.

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581125022
 
 
   > Maybe I have to downgrade lombok version.
   
   @JohnNiang no, using the latest Lombok version should be 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373851569
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   That depends on how you use it. Logically, they are the same. But `provided` is pointless. 

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373851621
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   And from the current `dependencies` in the root `pom.xml`, no lib is declared there.

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


With regards,
Apache Git Services

[GitHub] [skywalking] JohnNiang commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
JohnNiang commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373851304
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   @kezhenxu94 Should I move the `lombok` into root `dependencies` other than `dependencyManagement`.

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


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io commented on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581127226
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=h1) Report
   > Merging [#4309](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/bf38e43d3aadeec93a0b30f9b92e2226ff9e953c?src=pr&el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4309/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #4309   +/-   ##
   =======================================
     Coverage   26.58%   26.58%           
   =======================================
     Files        1170     1170           
     Lines       25479    25479           
     Branches     3627     3627           
   =======================================
     Hits         6773     6773           
     Misses      18103    18103           
     Partials      603      603
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=footer). Last update [bf38e43...b7e077a](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373889516
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   It should be better, and remove all Lombok in all sub modules

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373851621
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   And from the current `dependencies` in the root `pom.xml`, no lib is declared there.

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#discussion_r373851569
 
 

 ##########
 File path: pom.xml
 ##########
 @@ -275,6 +276,12 @@
                 <version>${powermock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
 
 Review comment:
   That depends on how you use it. Logically, they are the same. But `provided` is pointless. 

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


With regards,
Apache Git Services

[GitHub] [skywalking] JohnNiang commented on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
JohnNiang commented on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581124962
 
 
   Maybe I have to downgrade lombok version.

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


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581127226
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=h1) Report
   > Merging [#4309](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/bf38e43d3aadeec93a0b30f9b92e2226ff9e953c?src=pr&el=desc) will **increase** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4309/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4309      +/-   ##
   ==========================================
   + Coverage   26.58%   26.66%   +0.07%     
   ==========================================
     Files        1170     1170              
     Lines       25479    25479              
     Branches     3627     3627              
   ==========================================
   + Hits         6773     6793      +20     
   + Misses      18103    18083      -20     
     Partials      603      603
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4309/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `82.35% <0%> (+1.47%)` | :arrow_up: |
   | [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4309/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `31.46% <0%> (+3.37%)` | :arrow_up: |
   | [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4309/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `85.71% <0%> (+17.85%)` | :arrow_up: |
   | [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4309/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `77.04% <0%> (+18.03%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=footer). Last update [bf38e43...0be9469](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] JohnNiang commented on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
JohnNiang commented on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581143384
 
 
   > You should test the compiling locally first.
   
   The checks of test folder failed due to redundant dependency.

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


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 merged pull request #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
JaredTan95 merged pull request #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309
 
 
   

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


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581142985
 
 
   You should test the compiling locally first.

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


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4309: Move lombok dependency into root dependency manager for easier version management

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4309:  Move lombok dependency into root dependency manager for easier version management
URL: https://github.com/apache/skywalking/pull/4309#issuecomment-581127226
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=h1) Report
   > Merging [#4309](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/bf38e43d3aadeec93a0b30f9b92e2226ff9e953c?src=pr&el=desc) will **decrease** coverage by `<.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4309/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4309      +/-   ##
   ==========================================
   - Coverage   26.58%   26.57%   -0.01%     
   ==========================================
     Files        1170     1170              
     Lines       25479    25479              
     Branches     3627     3627              
   ==========================================
   - Hits         6773     6772       -1     
   - Misses      18103    18104       +1     
     Partials      603      603
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4309/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `66.66% <0%> (-1.29%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=footer). Last update [bf38e43...86ef6f6](https://codecov.io/gh/apache/skywalking/pull/4309?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services