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

Jump to bottom

Support pass context.Context to all methods #1034

Merged
merged 6 commits into from Feb 24, 2022
Merged

Support pass context.Context to all methods #1034

merged 6 commits into from Feb 24, 2022

Conversation

zchee
Copy link
Contributor

@zchee zchee commented Feb 14, 2022

Support pass context.Context to all methods.

  • is it performance related.

@zchee zchee changed the title Ctx aware Support pass context.Context to all methods Feb 14, 2022
@zchee
Copy link
Contributor Author

zchee commented Feb 14, 2022

@kanata2 You said

Actually, we no longer support v1.12, so we should delete webhooks_go112.go

#1032 (review)

So, Remove this file and always use NewRequestWithContext.

@zchee
all: support pass context.Context to all methods
0389f44 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
webhooks: remove go1.12 support
dacbdf8 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
misc: use NewRequestWithContext
d89251b 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
misc: use http.MethodXXX constant
1645ac2 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
Copy link
Contributor Author

zchee commented Feb 21, 2022

@kanata2 ping

kanata2
kanata2 reviewed Feb 24, 2022
View reviewed changes
Copy link
Member

@kanata2 kanata2 left a comment

Choose a reason for hiding this comment

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

Thanks!

chat.go Outdated Show resolved Hide resolved
webhooks.go Outdated Show resolved Hide resolved
@zchee
chat: add some BuildRequestContext methods for backwards compatibility
67b468f 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
webhook: remove unnecessary PostWebhookContextCustomHTTP function
741a1f5 
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
@zchee
Copy link
Contributor Author

zchee commented Feb 24, 2022

@kanata2 Sorry for that, I'd mistake :(

PTAL.

kanata2
kanata2 approved these changes Feb 24, 2022
View reviewed changes
Copy link
Member

@kanata2 kanata2 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@zchee zchee self-assigned this Feb 24, 2022
@zchee
Copy link
Contributor Author

zchee commented Feb 24, 2022

@kanata2 I'll merge later.

@zchee zchee merged commit ff619ea into slack-go:master Feb 24, 2022
@zchee zchee deleted the ctx-aware branch February 24, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kanata2 kanata2 kanata2 approved these changes

Assignees

@zchee zchee

Labels
api cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zchee @kanata2