Skip to content

chore: Remove allowed_strings in favour of Literal#1929

Closed
doublevcodes wants to merge 0 commit into
python-discord:mainfrom
doublevcodes:main
Closed

chore: Remove allowed_strings in favour of Literal#1929
doublevcodes wants to merge 0 commit into
python-discord:mainfrom
doublevcodes:main

Conversation

@doublevcodes
Copy link
Copy Markdown
Contributor

This PR removes the allowed_strings converter because we can use typing.Literal as a converter to the same effect

Comment thread bot/exts/info/doc/_cog.py Outdated
@Akarys42 Akarys42 requested a review from TizzySaurus November 1, 2021 21:31
Copy link
Copy Markdown
Contributor

@jchristgit jchristgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change, thank you!

Once the review by Tizzy is dealt with, this looks good to merge.

@jchristgit jchristgit enabled auto-merge November 3, 2021 21:10
@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Nov 5, 2021

I don't think Literal does a conversion to lowercase, but I don't think anyone was relying on that feature previously.

@Xithrius Xithrius added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) p: 3 - low Low Priority s: needs review Author is waiting for someone to review and approve t: enhancement Changes or improvements to existing features labels Nov 7, 2021
Copy link
Copy Markdown
Contributor

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just need the noqa removal.

Comment thread bot/exts/info/doc/_cog.py Outdated
Comment thread bot/exts/moderation/infraction/management.py Outdated
Comment thread bot/exts/moderation/infraction/management.py Outdated
@ichard26 ichard26 added the s: waiting for author Waiting for author to address a review or respond to a comment label Jan 2, 2022
@ichard26
Copy link
Copy Markdown
Contributor

ichard26 commented Jan 2, 2022

Hey @doublevcodes, it's been a while, do you plan on resuming work on this PR or should we put this up for grabs? Thank you!

@doublevcodes
Copy link
Copy Markdown
Contributor Author

Hey @doublevcodes, it's been a while, do you plan on resuming work on this PR or should we put this up for grabs? Thank you!

Hey, sorry for the delay, I can finish this in the coming week.

@ichard26 ichard26 removed the s: needs review Author is waiting for someone to review and approve label Jan 2, 2022
auto-merge was automatically disabled January 22, 2022 11:01

Head branch was pushed to by a user without write access

@doublevcodes
Copy link
Copy Markdown
Contributor Author

I fixed the noqas, but I did something and now I have 64 changed files 😅

@Bluenix2
Copy link
Copy Markdown
Contributor

Use git reset to remove that commit, then rebase your old commit off of upstream/main and force push?

@Xithrius
Copy link
Copy Markdown
Contributor

@doublevcodes are you still planning on fixing up the conflicts in this PR?

@MarkKoz MarkKoz self-assigned this Jun 4, 2022
@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Jun 4, 2022

I'm going to finish this up since there has not been a response for over a week. Seems almost done anyway.

@MarkKoz MarkKoz closed this Jun 4, 2022
@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Jun 4, 2022

Unfortunately I used the wrong command and ended up pushing my local main branch to doublevcodes/main, causing the diff to disappear and the PR to close. I can't push to that remote anymore since the PR is closed, and I have no way to reopen the PR. So I will have to just create a new PR unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) p: 3 - low Low Priority s: waiting for author Waiting for author to address a review or respond to a comment t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants