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