-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improvement] Add skywalking agent plugin #4852
Conversation
Hi,If this pr will generate more jars than before,there need add license files under dolphinscheduler-dist/releases. |
Please check. |
I have a question for the DolphinScheduler PMC, why you are requiring all Apache 2.0 licenses in your dist folder? SkyWalking and DolphinScheduler are in the same foundation, we are using the same License. Basically, you just need to mention Apache SkyWalking is included in your distribution, but I can't find your dist LICENSE change, where is it? |
https://github.com/apache/incubator-dolphinscheduler/blob/1.3.6-prepare/dolphinscheduler-dist/release-docs/LICENSE This license file should be updated by adding SkyWalking and its dependencies(agent core's) to it. But I highly doubt, it is meaningful to add all the duplicated Apache 2.0 licenses. |
Thanks for tips and help,If the license of new dependencies are standard apache 2.0 license,It's not necceary to add the new license file under the folder dolphinscheduler-dist/release-docs,but it need to be added under dolphinscheduler-dist/release-docs/LICENSE. |
@hailin0 I think you still miss to update https://github.com/apache/incubator-dolphinscheduler/blob/1.3.6-prepare/dolphinscheduler-dist/release-docs/LICENSE |
One question, where could I read your documentation? I could help on checking whether it is accurate from the SkyWalking's perspective. |
it's here apache/dolphinscheduler-website#299 |
SonarCloud Quality Gate failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to me. Thanks @hailin0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Tips
What is the purpose of the pull request
Brief change log
Verify this pull request