fix: address error logging regression #204
Merged
Conversation
This file contains 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
StaticRocket
force-pushed
the
fix-error-logging
branch
from
2deed8d
to
c655881
Compare
|
@aquelemiguel any input on this since it is more of ideological dilemma? |
|
Hi @StaticRocket, as always thank you for your love for the project. I haven't been active on here for a while, I need a bit to recontextualize myself. I'll get back to you this week still. 🙂 |
joao-conde
reviewed
Oct 23, 2022
|
@StaticRocket Your idea looks good. While we discuss how to handle the errors in @joao-conde's review, personally you can undraft this. 🙂 |
StaticRocket
force-pushed
the
fix-error-logging
branch
from
e65008f
to
73af4ea
Compare
StaticRocket
force-pushed
the
fix-error-logging
branch
from
03b454f
to
31fe9b3
Compare
joao-conde
approved these changes
Oct 25, 2022
aquelemiguel
approved these changes
Oct 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Potential solution to #203
This makes create_embed_response clobber previous responses by default. I believe this approach is closer to intended behavior, but an alternative that some might like better is if instead of clobbering the original response it posts as a followup. Looking for feedback here.
I almost wish serenity bit the bullet here and tracked whether an interaction received a response locally. Unfortunately this was the nicest way to do this without adding another call to the discord api.