Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[test/plugin] immigrate test play framework scenario #4128

Merged
merged 19 commits into from
Dec 26, 2019
Merged

[test/plugin] immigrate test play framework scenario #4128

merged 19 commits into from
Dec 26, 2019

Conversation

aiyanbo
Copy link
Contributor

@aiyanbo aiyanbo commented Dec 25, 2019 •
edited

Please answer these questions before submitting pull request

@wu-sheng
Copy link
Member

Please follow #4121, and make play framework as default plugin.

@wu-sheng wu-sheng added this to the 7.0.0 milestone Dec 25, 2019
@wu-sheng wu-sheng added agent Language agent related. 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 Dec 25, 2019
@codecov-io
Copy link

codecov-io commented Dec 25, 2019 •
edited

Codecov Report

Merging #4128 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4128      +/-   ##
==========================================
+ Coverage    27.2%   27.23%   +0.02%     
==========================================
  Files        1141     1141              
  Lines       25057    25057              
  Branches     3625     3625              
==========================================
+ Hits         6818     6824       +6     
+ Misses      17635    17627       -8     
- Partials      604      606       +2
Impacted Files Coverage Δ
...ng/apm/plugin/play/v2x/HttpFiltersInterceptor.java 76.92% <ø> (ø)
.../skywalking/apm/plugin/play/v2x/TracingFilter.java 80.55% <ø> (ø)
...apm/agent/core/remote/GRPCStreamServiceStatus.java 29.16% <0%> (-25.01%) ⬇️
.../core/remote/ServiceAndEndpointRegisterClient.java 44.94% <0%> (+13.48%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea9acc5...76a3bfa. Read the comment docs.

@aiyanbo aiyanbo changed the title WIP [test/plugin] immigrate test play framework scenario [test/plugin] immigrate test play framework scenario Dec 25, 2019
wu-sheng
wu-sheng reviewed Dec 25, 2019
View reviewed changes
Copy link
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.

plugins-test.yaml is not updated, this test will not run in the CI process.

@wayilau
Copy link
Member

wayilau commented Dec 26, 2019

@aiyanbo Please resolve the conflicts first.

@wu-sheng
Copy link
Member

Even I resolved the conflicts, the CI still shows error. Please recheck.

@wu-sheng
Copy link
Member

@aiyanbo Are you in the SkyWalking slack? There is one feedback about this Play plugin from India. If you have interests and time, please take a look. I am not familiar with the Play ecosystem. Can't help much.

@aiyanbo
Copy link
Contributor Author

aiyanbo commented Dec 26, 2019

@aiyanbo Are you in the SkyWalking slack? There is one feedback about this Play plugin from India. If you have interests and time, please take a look. I am not familiar with the Play ecosystem. Can't help much.

Yes, i was joined SkyWalking channel. I will help others using SkyWalking in PlayFramework.

@wu-sheng
Copy link
Member

Yes, i was joined SkyWalking channel. I will help others using SkyWalking in PlayFramework

Very appreciated.

wayilau
wayilau reviewed Dec 26, 2019
View reviewed changes
test/plugin/scenarios/play-scenario/build.sbt
*
*/

name := """play-scenario"""
Copy link
Member

Choose a reason for hiding this comment

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

Is this a mistake ? @aiyanbo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is project name in SBT. And the """""" style is Scala Strip Strings

Copy link
Member

Choose a reason for hiding this comment

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

I got it.

@wayilau
Copy link
Member

wayilau commented Dec 26, 2019

LGTM.
Waiting other committers review.

@wu-sheng wu-sheng merged commit 4aac329 into apache:master Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wayilau wayilau wayilau left review comments

@dmsolr dmsolr dmsolr left review comments

@wu-sheng wu-sheng wu-sheng approved these changes

@arugal arugal Awaiting requested review from arugal

Assignees
No one assigned
Labels
agent Language agent related. 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
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@aiyanbo @wu-sheng @codecov-io @wayilau @dmsolr