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

change the Filter parameter from *http.Request to *gin.Context #3070 #4375

Closed
wants to merge 10 commits into from

Conversation

rehanpfmr
Copy link
Contributor

change #3070

@rehanpfmr rehanpfmr requested a review from MrAlias as a code owner October 3, 2023 12:32
@rehanpfmr rehanpfmr requested a review from a team October 3, 2023 12:32
Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

Please do not delete instrgen/docs/flow.png.

CHANGELOG.md Outdated Show resolved Hide resolved
@dmathieu
Copy link
Member

dmathieu commented Oct 3, 2023

Note that you don't need to close/reopen PRs for every change made. If you push a new commit (or amend an existing commit) to the same branch on your fork, the PR will be updated.

@rehanpfmr
Copy link
Contributor Author

I had unexplainable issue fixing GO package changes, So i had to just close and reopen new one. I hope I will no longer have to do it again.

@dmathieu
Copy link
Member

dmathieu commented Oct 3, 2023

The instrgen/docs/flow.png file is currently deleted in your PR. Could you bring it back?

@rehanpfmr
Copy link
Contributor Author

@dmathieu the instrgen/docs/flow.png is already there. However it shows as "Unable to render rich display Invalid image source"
04e9f5b

@rehanpfmr
Copy link
Contributor Author

please help fix the image

@dmathieu
Copy link
Member

dmathieu commented Oct 4, 2023

Yes, it's already on the main branch of the repository. But if you look at the diff for your PR (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/4375/files), you will see it changes when it shouldn't.

@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #4375 (86bb9a7) into main (f567afc) will not change coverage.
The diff coverage is 100.0%.

❗ Current head 86bb9a7 differs from pull request most recent head c7b00b6. Consider uploading reports for the commit c7b00b6 to get more accurate results

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4375   +/-   ##
=====================================
  Coverage   82.3%   82.3%           
=====================================
  Files        144     144           
  Lines      10002   10002           
=====================================
  Hits        8232    8232           
  Misses      1632    1632           
  Partials     138     138           
Files Coverage Δ
...ation/github.com/gin-gonic/gin/otelgin/gintrace.go 83.5% <100.0%> (ø)
...ntation/github.com/gin-gonic/gin/otelgin/option.go 100.0% <ø> (ø)

@rehanpfmr
Copy link
Contributor Author

Can this be merged?

@pellared
Copy link
Member

pellared commented Oct 14, 2023

in branch of the repository. But if you look at the diff for your PR (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/4375/files), you will see it chan

This issue is not addressed.

This branch is out-of-date with the base branch

You need to make sure to keep the branch up to date.

rehanpfmr and others added 9 commits October 16, 2023 11:12
@rehanpfmr
Copy link
Contributor Author

@pellared
Copy link
Member

Superseded by #4444

@pellared pellared closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants