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

Upgrade httpsnoop v1.0.1 => v1.0.3 #235

Closed
wants to merge 2 commits into from
Closed

Upgrade httpsnoop v1.0.1 => v1.0.3 #235

wants to merge 2 commits into from

Conversation

malt3
Copy link

@malt3 malt3 commented Aug 17, 2022

Fixes #234

Summary of Changes

  1. Upgrade httpsnoop

@amustaque97
Copy link

Can you please fix the CI pipeline failure issue? I will also take a look if updating the httpsnoop version is a breaking change or not.

Thank you

@malt3
Copy link
Author

malt3 commented Aug 18, 2022

Can you please fix the CI pipeline failure issue?

Here you go #236

@amustaque97
Copy link

amustaque97 commented Aug 21, 2022

Hi @malt3, oops I should write it briefly. You need to fix the pipeline in the current PR.
Here is the required diff to understand file changes better

diff --git a/handlers_go18_test.go b/handlers_go18_test.go
index d8e6321..ec227cc 100644
--- a/handlers_go18_test.go
+++ b/handlers_go18_test.go
@@ -1,3 +1,4 @@
+//go:build go1.8
 // +build go1.8
 
 package handlers

If you find it difficult to understand. Save the above diff output in a file let's say patch.txt and apply the patch git apply patch.txt. Add a commit message and push the changes.
You can close other PR #236

Indeed this is not a breaking change, we can request the actual maintainer to merge this PR.

@malt3
Copy link
Author

malt3 commented Aug 21, 2022

It is very weird for me to resolve existing problems in this unrelated PR. I also question wether you are an actual maintainer of this project. Maintainers can edit the PR so feel free to apply the exact patch you proposed to me yourself.

@amustaque97
Copy link

It is very weird for me to resolve existing problems in this unrelated PR.

I agree but keeping in mind that the original maintainer is quite busy and it's better to accommodate that minor change in the same PR.

I also question whether you are an actual maintainer of this project. Maintainers can edit the PR so feel free to apply the exact patch you proposed to me yourself.

No, I'm not the actual maintainer that's why I'm requesting you. 🙂

@malt3
Copy link
Author

malt3 commented Aug 21, 2022

I see. 😄

Sorry for making this complicated initially.

Copy link

@amustaque97 amustaque97 left a comment

Choose a reason for hiding this comment

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

Hi @elithrar please merge this PR.

Thank you

@coreydaley
Copy link
Contributor

Completed via #241

@coreydaley coreydaley closed this Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[question] upgrade httpsnoop
3 participants