Skip to content

Conversation

@silasary
Copy link
Member

@silasary silasary commented Jun 5, 2023

Pull Request Type

  • Feature addition
  • Bugfix
  • Documentation update
  • Code refactor
  • Tests improvement
  • CI/CD pipeline enhancement
  • Other: [Replace with a description]

Description

We frequently recieve support requests where provided code snippits start with the line "import discord" and inevitably don't work.
Due to the nature of humans generally importing stuff alphabetically, we can do runtime check for this import, and yell at them.

Changes

Related Issues

Test Scenarios

Python Compatibility

  • I've ensured my code works on Python 3.10.x
  • I've ensured my code works on Python 3.11.x

Checklist

  • I've run the pre-commit code linter over all edited files
  • I've tested my changes on supported Python versions
  • I've added tests for my code, if applicable
  • I've updated / added documentation, where applicable

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.09 ⚠️

Comparison is base (4bc6ae7) 60.27% compared to head (938d6c2) 60.19%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           stable    #1440      +/-   ##
==========================================
- Coverage   60.27%   60.19%   -0.09%     
==========================================
  Files         139      139              
  Lines       14705    14708       +3     
==========================================
- Hits         8863     8853      -10     
- Misses       5842     5855      +13     
Impacted Files Coverage Δ
interactions/__init__.py 88.88% <66.66%> (-11.12%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@LordOfPolls
Copy link
Contributor

Alternate idea. Move this check to client's sanity check method

@silasary silasary force-pushed the warn-dual-libraries branch 2 times, most recently from 16b806a to 49edc83 Compare June 6, 2023 09:58
@silasary
Copy link
Member Author

silasary commented Jun 6, 2023

Considered, implemented, then reverted upon further consideration 😂

Take this thread as an example: https://discord.com/channels/789032594456576001/1110958056327626753/1110959282213621860

import discord
from discord import app_commands
from discord.ext import commands
from interactions import slash_command, SlashContext, Permissions, slash_default_member_permission, Extension
...
bot = commands.Bot (command_prefix="*", intents = discord.Intents.all())
slash = slash_command(bot, sync_commands=True)
command_count = 0
staff = Permissions.MANAGE_ROLES

class Player(commands.Cog):
    def __init__(self, bot):
        self.bot = bot

    @slash_command(name="X", description="X")
    @slash_default_member_permission(staff)
    async def hello(self, ctx: SlashContext):
        await ctx.send(f"Hey {ctx.author.mention}! Test")


bot.add_cog(Player(bot))

@bot.event
async def on_ready():
    print(f"Bot conectado como {bot.user.name}")
    await slash.sync_all_commands()

token = os.getenv('TOKEN')
bot.run(token)

They're using interactions decorators on a d.py client. They're still going to run into issues, but they won't ever call sanity check.

@LordOfPolls
Copy link
Contributor

LordOfPolls commented Jun 6, 2023

Id argue that's extreme user error at that point. But fair point, then I have one more suggestion. Put it in the root init

@silasary
Copy link
Member Author

silasary commented Jun 6, 2023

That was my initial plan, but it felt weird.

@LordOfPolls
Copy link
Contributor

It's standard practice for checks like this. Const feels weird, imo

@silasary
Copy link
Member Author

silasary commented Jun 6, 2023

Understandable. I'll move it to init

@LordOfPolls LordOfPolls merged commit ec7ef71 into stable Jun 9, 2023
@LordOfPolls LordOfPolls deleted the warn-dual-libraries branch June 9, 2023 06:45
@LordOfPolls
Copy link
Contributor

oh fml. this targetted stable... dammit sil, i trusted you

LordOfPolls pushed a commit that referenced this pull request Jun 9, 2023
* feat:  Warn if people try to run d.py alongside interactions

* refactor: Move d.py check to __init__.py
@silasary
Copy link
Member Author

Ooops. I blame the fact that stable and unstable were at the same place when I made the PR.

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.

4 participants