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

Threads reloaded #1058

Merged
merged 24 commits into from Feb 17, 2022
Merged

Threads reloaded #1058

merged 24 commits into from Feb 17, 2022

Conversation

FedorLap2006
Copy link
Collaborator

@FedorLap2006 FedorLap2006 commented Dec 18, 2021

So, seeing still ongoing problem with lack of threads implementation, I decided to make my own PR on them.
This PR should include everything that's on threads currently.

This PR also includes some code from #982 and #922 (it is meant to be continuation of those two, but written from scratch). So credits to the authors.

The roadmap

  • Gateway
  • REST
  • State
    • CRUD
    • Membership

@FedorLap2006 FedorLap2006 changed the title Threads Threads reloaded Dec 18, 2021
@FedorLap2006
Copy link
Collaborator Author

🚧 This is still under development

@FedorLap2006
Copy link
Collaborator Author

Woah, that went pretty fast. Anyways, should be everything now.

@FedorLap2006 FedorLap2006 marked this pull request as ready for review December 19, 2021 12:58
@FedorLap2006
Copy link
Collaborator Author

FedorLap2006 commented Dec 19, 2021

@CarsonHoffman Take a look

@orakaro
Copy link

orakaro commented Dec 22, 2021

Huge ++ to this work, and I would love to have Thread support on DiscordGo also. Appreciate if @CarsonHoffman could make this happen.

structs.go Outdated Show resolved Hide resolved
state.go Outdated Show resolved Hide resolved
state.go Outdated Show resolved Hide resolved
restapi.go Outdated Show resolved Hide resolved
@orakaro
Copy link

orakaro commented Jan 13, 2022

Bumped! Thanks for your work and how is this going? I am looking forward to have Thread supported any time soon!

@FedorLap2006
Copy link
Collaborator Author

Bumped! Thanks for your work and how is this going? I am looking forward to have Thread supported any time soon!

Well, I think by now everything is finished, just waiting when @CarsonHoffman does review on this.

restapi.go Outdated Show resolved Hide resolved
state.go Outdated Show resolved Hide resolved
state.go Outdated Show resolved Hide resolved
state.go Outdated Show resolved Hide resolved
events.go Outdated Show resolved Hide resolved
@trag-stripe
Copy link

@CarsonHoffman and @FedorLap2006 👋 - checking in if this pull is ready to go?
Thanks for building this project for the community and making all our lives easier!

@FedorLap2006
Copy link
Collaborator Author

@CarsonHoffman and @FedorLap2006 wave - checking in if this pull is ready to go? Thanks for building this project for the community and making all our lives easier!

Almost. If I didn't forget anything then it's gonna be only the LastMessageID thing and that will be it.

@omegasome
Copy link

So we're just waiting for @CarsonHoffman to provide their input on the LastMessageID thing?

@FedorLap2006
Copy link
Collaborator Author

So we're just waiting for @CarsonHoffman to provide their input on the LastMessageID thing?

Yes.

@srikanthrc
Copy link

This would be really cool extension to what the library already allows for. Would love to see this land sooner than later. @CarsonHoffman any chance we can bump this up in the queue of things to get to? thanks in advance,

@FedorLap2006
Copy link
Collaborator Author

@CarsonHoffman I think it's ready for the final review.

@shrapx
Copy link

shrapx commented Feb 4, 2022

@FedorLap2006 I found a missing threadID assignment intended here:
https://github.com/FedorLap2006/discordgo/blob/threads/restapi.go#L2223

(added snippet)

func (s *Session) WebhookThreadExecute(webhookID, token string, wait bool, threadID string, data *WebhookParams) (st *Message, err error) {
  return s.webhookExecute(webhookID, token, wait, "", data)
}

@FedorLap2006
Copy link
Collaborator Author

After deeper reading of Discord's documentation and our current source code, I've come to the conclusion that Carson with right, about LastMessageID and it should not be changed. On other hand I will open a PR so it would be easier to work this out

@FedorLap2006
Copy link
Collaborator Author

FedorLap2006 commented Feb 17, 2022

There are a couple of problems (it deletes some threads rest methods) with merge of master.
I'm going to resolve them in a couple of hours.
Please don't update the branch until I fix these issues, they will break your code.

@FedorLap2006 FedorLap2006 merged commit 992358e into bwmarrin:master Feb 17, 2022
@FedorLap2006 FedorLap2006 added this to the v0.24.0 milestone Feb 27, 2022
@FedorLap2006 FedorLap2006 added the feature Feature implementation label Feb 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants