Skip to content

Remove imported classes and add ci scripts for plugin import check.#2881

Merged
wu-sheng merged 11 commits intoapache:masterfrom
dmsolr:remove-imported-classes
Jun 16, 2019
Merged

Remove imported classes and add ci scripts for plugin import check.#2881
wu-sheng merged 11 commits intoapache:masterfrom
dmsolr:remove-imported-classes

Conversation

@dmsolr
Copy link
Copy Markdown
Member

@dmsolr dmsolr commented Jun 15, 2019

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.

Imported 3rd-party classes unless JDK or ByteBuddy maybe trigger agent crash. So I remove it. And provide a shell to check it. But the script is very weak.

Copy link
Copy Markdown
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License headers should be added.

Yet this is not perfect because one can still use the full qualified class names out of the scopes.

Comment thread PluginImportedCheck.sh Outdated
Comment thread Jenkinsfile Outdated
}
}

stage('Check 3rd-party classes imported') {
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng Jun 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Named as Check agent plugin instrumentation imports

@wu-sheng
Copy link
Copy Markdown
Member

Grant shell to execute right, run commit the shell

@wu-sheng wu-sheng added agent Language agent related. bug Something isn't working and you are sure it's a bug! feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. test Test requirements about performance, feature or before release. labels Jun 16, 2019
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move all shells to tools/check/agent/plugin folder.

Comment thread tools/check/agent/plugin/PluginImportedCheck.sh Outdated
@wu-sheng wu-sheng changed the title Remove imported classes Remove imported classes and add ci scripts for plugin import check. Jun 16, 2019
@wu-sheng wu-sheng added this to the 6.2.0 milestone Jun 16, 2019
@wu-sheng wu-sheng requested a review from ascrutae June 16, 2019 05:09
Comment thread tools/check/agent/plugin/PluginImportedCheck.sh
Comment thread tools/check/agent/plugin/PluginImportedCheck.sh Outdated
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @kezhenxu94 @ascrutae please recheck.

Comment thread tools/check/agent/plugin/PluginImportedCheck.sh Outdated
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for update.

@wu-sheng
Copy link
Copy Markdown
Member

@dmsolr Thanks for providing this valuable scripts in CI. @ascrutae I am merging this CI scripts, which could very helpful for our further plugin review.

@wu-sheng wu-sheng merged commit dbe7da4 into apache:master Jun 16, 2019
@dmsolr dmsolr deleted the remove-imported-classes branch July 16, 2019 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. bug Something isn't working and you are sure it's a bug! feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. test Test requirements about performance, feature or before release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants