Skip to content
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

Add validation of option names and descriptions #1271

Merged
merged 12 commits into from Apr 18, 2022

Conversation

Middledot
Copy link
Member

Summary

Add validation checks for options.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...)

discord/commands/core.py Outdated Show resolved Hide resolved
discord/commands/core.py Outdated Show resolved Hide resolved
discord/commands/core.py Outdated Show resolved Hide resolved
discord/commands/core.py Outdated Show resolved Hide resolved
discord/commands/core.py Outdated Show resolved Hide resolved
krittick and others added 6 commits April 16, 2022 23:54
Co-authored-by: krittick <ben@krittick.net>
Co-authored-by: krittick <ben@krittick.net>
Co-authored-by: krittick <ben@krittick.net>
Co-authored-by: krittick <ben@krittick.net>
Co-authored-by: krittick <ben@krittick.net>
Lulalaby
Lulalaby previously approved these changes Apr 17, 2022
@Lulalaby Lulalaby enabled auto-merge (squash) April 17, 2022 22:07
Co-Authored-By: krittick <935885+krittick@users.noreply.github.com>
@Middledot
Copy link
Member Author

“in locale …” is supposed to be optionally added since the main name doesn't have a locale attached, and with how the if/else is placed, description-less errors happen

image

It can be reformatted so that it adds it to the string afterwards though since that solves formatting issues

Lulalaby
Lulalaby previously approved these changes Apr 17, 2022
@Lulalaby
Copy link
Member

Good job

@Lulalaby Lulalaby requested review from krittick and Lulalaby and removed request for krittick April 17, 2022 23:33
@krittick krittick added the feature Implements a feature label Apr 17, 2022
@krittick krittick added this to In progress in v2.0 via automation Apr 17, 2022
@krittick krittick added this to the v2.0 milestone Apr 17, 2022
@Dorukyum
Copy link
Member

The "adding locale to error message" part is done multiple times. What if we did the following:

  1. Instead of raising the chosen error, set it to a var (e.g. error = ValidationError)
  2. At the end check for the locale, add it to the error message if it's passed, and raise the variable error with the set message

1 similar comment
@Dorukyum

This comment was marked as duplicate.

seems to work
Copy link
Member

@Dorukyum Dorukyum 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, thanks

@Lulalaby Lulalaby merged commit 1dd3974 into Pycord-Development:master Apr 18, 2022
v2.0 automation moved this from In progress to Done Apr 18, 2022
@Middledot Middledot deleted the l10-things branch April 18, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Implements a feature
Projects
Status: No status
v2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants