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

feat(GuildForumThreadManager): add pinned getter #10655

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SSardorf
Copy link

Please describe the changes this PR makes and why it should be merged:
Given that there can only be one pinned thread per guild forum, a helper getter is added to provide an easier way to retrieve it.

Status and versioning classification:

  • This PR changes the library's interface (methods or parameters added)

@SSardorf SSardorf requested a review from a team as a code owner December 11, 2024 13:14
Copy link

vercel bot commented Dec 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Dec 11, 2024 1:14pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Dec 11, 2024 1:14pm

@Jiralite
Copy link
Member

Pinned posts are decided with a channel flag—it would be very easy for Discord to say "Hey, let's make the maximum pinned posts 2!" and then suddenly this will be the source of confusion as it may return one or the other post only, which will result in an eventual deprecation and breaking change to remove it in the future.

I just don't see this as something the library should implement.

@SSardorf
Copy link
Author

Pinned posts are decided with a channel flag—it would be very easy for Discord to say "Hey, let's make the maximum pinned posts 2!" and then suddenly this will be the source of confusion as it may return one or the other post only, which will result in an eventual deprecation and breaking change to remove it in the future.

I just don't see this as something the library should implement.

Or we change it to filter on the Pinned channel flag instead of find in the event of an eventual change in the maximum amount of pinned posts

@Jiralite
Copy link
Member

In the event of an eventual change in the maximum amount of pinned posts, modifying find() to filter() is a breaking change. The return type changing to a possible thread to a collection is not backwards-compatible.

If this is to happen, it needs to be a filter now. That being said, a filter returning only one result is strange.

I am thinking long-term here, and it seems something better left to consumers on how they want to get a pinned post.

@Jiralite Jiralite added this to the discord.js 15.0.0 milestone Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Denied
Development

Successfully merging this pull request may close these issues.

2 participants