-
-
Notifications
You must be signed in to change notification settings - Fork 475
refactor: Rewrite the role tags mess #2708
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
base: master
Are you sure you want to change the base?
refactor: Rewrite the role tags mess #2708
Conversation
20d75bb
to
696dcbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really easy to make backwards compatible, can we just do that and deprecate all the compatibility methods?
Yeah maybe i'll try, but it sucks when I reused method names to become properties |
What are your thoughts on adding something like an enum, e.g. |
As we previously talked on dcs, the role type enum is the best way. Like plun said, deprecated all current methods and just create the type on role. But the docs should clarify that it's calculated since it's not returned by discord |
0c18960
to
7e7d40e
Compare
7e7d40e
to
5810fa3
Compare
27a15ee
to
54f1baa
Compare
Here is a handy snippet for testing this: import logging
import os
from datetime import datetime, UTC
from dotenv import load_dotenv
import discord
load_dotenv()
logging.basicConfig(level=logging.DEBUG)
TOKEN = os.getenv("TOKEN_3")
bot = discord.Bot()
@bot.slash_command()
async def test_roles(ctx: discord.ApplicationContext, role: discord.Role):
await ctx.defer()
if not ctx.guild:
return await ctx.respond("This command must be used in a guild.")
fetched_role = await ctx.guild.fetch_role(role.id)
await ctx.respond(f"""```
Role:
{role!r}
Role tags:
{role.tags!r}
Role tags data:
{role.tags._data if role.tags else None}
Fetched role:
{fetched_role!r}
Fetched role tags:
{fetched_role.tags!r}
Fetched role tags data:
{fetched_role.tags._data if fetched_role.tags else None}
```""")
bot.run(TOKEN) |
Do not merge yet |
Missing testing is only twitch/ytb roles, everything else is tested. |
def is_guild_connections_role(self) -> bool: | ||
"""Whether the role is a guild connections role. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait this could be removed if this pr is merged before 2.7 since it was added after 2.6.1. Same for is_available_for_purchase
c41d7ee
to
2dc7e65
Compare
Any news on this topic ? |
@Lulalaby Any news on this btw ? |
not yet :( |
Summary
Rewrite the role tags mess.

Information
examples, ...).
Checklist
type: ignore
comments were used, a comment is also left explaining why.