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

Components Support #956

Closed
wants to merge 5 commits into from
Closed

Conversation

fletcherist
Copy link

@fletcherist fletcherist commented Jun 22, 2021

@phenpessoa
Copy link
Contributor

Isn't this already being worked on #933?

@FedorLap2006
Copy link
Collaborator

Same question.

@fletcherist
Copy link
Author

fletcherist commented Jun 23, 2021

@Pedro-Pessoa not exactly. The implementation by @FedorLap2006 breaks backwards-compatibility and contains breaking changes. This, on the other hand, can be released as minor patch.

https://github.com/bwmarrin/discordgo/pull/933/files#diff-20a180d42cb406f9578ddcd7228e219fb98d12f9f83648aada9e18a5be1cf207L169
Screen Shot 2021-06-23 at 1 19 26 PM

So i think the best way is to treat this as yet another approach

@CarsonHoffman
Copy link
Collaborator

The changes introduced in the existing PR serve to more closely align the library with the state of the world on Discord's end after they made some breaking changes; it's not worth having incorrectly-named things years down the line in the name of backwards compatibility on a feature that has yet to appear in any tagged release (i.e. it's not really a breaking change at all).

I don't mean to shoot you down too hard, but at this point, I see no reason that the other PR cannot be moved ahead with, so if you'd like to see buttons in the library as soon as possible, I'd suggest contributing a bit of your time as a review on that PR rather than duplicating effort.

@fletcherist
Copy link
Author

@CarsonHoffman okay I clearly got your idea. Closing this tho, thanks for explanation

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.

None yet

4 participants