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.

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

feat: add support for go-restful v3 #968

Merged
merged 3 commits into from Jun 3, 2021
Merged

feat: add support for go-restful v3 #968

merged 3 commits into from Jun 3, 2021

Conversation

luqmansen
Copy link
Contributor

@cla-checker-service
Copy link

cla-checker-service bot commented Jun 3, 2021

💚 CLA has been signed

@elastic-apm-tech elastic-apm-tech added this to In Progress in APM-Agents (OLD) Jun 3, 2021
@apmmachine
Copy link
Collaborator

apmmachine commented Jun 3, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #968 updated

  • Start Time: 2021-06-03T09:28:13.132+0000

  • Duration: 30 min 16 sec

  • Commit: bf6c091

Test stats 🧪

Test Results
Failed 0
Passed 10739
Skipped 266
Total 11005

Trends 🧪

Image of Build Times

Image of Tests

@luqmansen
Copy link
Contributor Author

luqmansen commented Jun 3, 2021

Sorry, forgot to sign the contributor agreement, now I have signed it, how to ask the bot to recheck my PR?

edit: done

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Thanks @luqmansen! Just a couple of minor issues. Please run make fmt update-modules scripts/Dockerfile-testing, then it should be good to merge.

module/apmrestfulv3/filter_test.go Outdated Show resolved Hide resolved
module/apmrestfulv3/go.mod Outdated Show resolved Hide resolved
@luqmansen
Copy link
Contributor Author

Hey, I updated the PR, can you please review it again? Thank you @axw

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I'll just need to get it to run through CI now.

@axw
Copy link
Member

axw commented Jun 3, 2021

jenkins run the tests please

@axw
Copy link
Member

axw commented Jun 3, 2021

@luqmansen CI looks good except that it fails on older versions of Go that don't support modules. Can you please add a build tag to each file in apmrestfulv3 except for doc.go: // +build go1.11. If you're unfamiliar with using build tags, here's an example for reference:

// +build go1.14
package apmgopgv10 // import "go.elastic.co/apm/module/apmgopgv10"

@luqmansen
Copy link
Contributor Author

Sure, will update this soon

@axw
Copy link
Member

axw commented Jun 3, 2021

🎉 thanks again @luqmansen!

@axw axw merged commit 5d7a990 into elastic:master Jun 3, 2021
APM-Agents (OLD) automation moved this from In Progress to Done Jun 3, 2021
@axw axw mentioned this pull request Jun 3, 2021
@luqmansen
Copy link
Contributor Author

Thank you! @axw

stuartnelson3 pushed a commit that referenced this pull request Jul 30, 2021
* feat: add support for go-restful v3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants