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

Fixes SocketMode support in slacktest #1247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

prologic
Copy link

@prologic prologic commented Dec 13, 2023

  • Add support for /apps.conenctions.open endpoint for slacktest
  • Add missing ping control message support for test server

Fixes #1244

@prologic prologic changed the title master Fixes SocketMode support in slacktest Dec 13, 2023
@dnck
Copy link

dnck commented Feb 4, 2025

Where's this PR stand? It's been open for quite some time, and I'm currently working on writing some tests against my app's usage of the websocket client, and I'm seeing similar issue as addressed in this open PR. I'll try it out the code changes and see how it goes.

@nlopes nlopes self-requested a review February 6, 2025 12:30
@nlopes
Copy link
Collaborator

nlopes commented Feb 6, 2025

👋 I'm currently going over a review of most of the PRs here.

The only thing I'm not super keen in this PR is the introduction of go.mills.io/logger. I don't have an objection to the library per se but I'd rather not introduce something not updated in 3 years and already a fork of something that was archived.

@prologic what's your take here? Can we do something instead with logrus or the likes which are better maintained?

@dnck
Copy link

dnck commented Feb 7, 2025

@nlopes that makes sense. If you're already using logrus, why introduce yet another logging framework!

@prologic
Copy link
Author

prologic commented Feb 8, 2025

Sorry folks, I've read these comments, but I haven't had the band with yet to respond. I'll have a look today if I have some time.

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.

Fix SocketMode support in slacktest
3 participants