-
-
Notifications
You must be signed in to change notification settings - Fork 751
helpdm for participants #1629
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
Merged
Merged
helpdm for participants #1629
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
a87143c
nothing to see here
f187868
Add caches required for help-dm
GDWR 9dd194a
Add help dm feature
GDWR c808ca0
Merge pull request #1 from JakeM0001/griff/helpdm
jacobmonck 8dac3ec
Add helpdm participating embed
2d5247e
Remove embed test
f05169e
Add docstring to commands
849b0bf
Fix failed linting
016b542
Remove useless else and if statement
0799f24
Change mention format
jacobmonck cc0f9ea
Make toggle command one command instead of 2
fd9ec20
Cleanup indentation and participant dm
0dfaf21
Make helpdm command more concise
f9d57b4
Fix reverted change
6b98aaf
Change discord.py colour to constants colour
6d801a6
Edit ignore close messages if statement
d5ed4a1
Merge branch 'main' into jake/helpdm
ChrisLovering 63682ce
Add bool converter to allow a variety of inputs
e9b10cf
Merge remote-tracking branch 'origin/jake/helpdm' into jake/helpdm
4b73cc2
Add bool converter to allow a variety of inputs
a2ecf1c
Edit indentation
jacobmonck 093f673
Merge branch 'main' into jake/helpdm
ChrisLovering File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Aren't sets in redis more appropriate for this sort of thing? It would remove the need to serialise and deserialise (unless you count str -> int as deserialisation). It wouldn't even need to retrieve all items since there are commands to check set membership.
Uh oh!
There was an error while loading. Please reload this page.
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.
Async Rediscache doesn't support sets. We were thinking about using sets but when we figured out it doesn't support it, we switched to that.
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.
aioredis does support it and async-redis cache can trivially be extended to support it (yes I think it's worth doing that).
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.
Note I'm not suggesting implementing the set operations with all the type conversion stuff. Just a basic SADD call or whatever that will store a string as given.
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.
I'm not too familiar with Redis can you please give more information about how I would implement this? Also I think it would be best to do this as a separate feature.
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.
That's alright. Let's just leave your implementation for now. I can try to merge set support into async-rediscache in the future.
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.
Alright, sounds good!