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

feat: new method for choice fail #2622

Merged
merged 8 commits into from
Nov 3, 2024

Conversation

antazoey
Copy link
Contributor

@antazoey antazoey commented Oct 4, 2023

Refactors so the failing happens in a new public method.

Ensure each step in CONTRIBUTING.rst is complete by adding an "x" to
each box below.

If only docs were changed, these aren't relevant and can be removed.
-->

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

Copy link
Collaborator

@AndreasBackx AndreasBackx left a comment

Choose a reason for hiding this comment

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

This seems like a fine choice. Could you follow through with the rest of the checklist like documentation and tests?

src/click/types.py Outdated Show resolved Hide resolved
src/click/types.py Outdated Show resolved Hide resolved
@AndreasBackx AndreasBackx mentioned this pull request Oct 26, 2024
39 tasks
@AndreasBackx AndreasBackx added this to the 8.2.0 milestone Oct 26, 2024
@antazoey antazoey force-pushed the feat/fail-method branch 2 times, most recently from 327e06e to 0fb1d4c Compare October 26, 2024 15:32
@antazoey
Copy link
Contributor Author

antazoey commented Oct 26, 2024

I did my best to complete the checkboxes. Notes:

  • I wasn't sure where to put the test as there were no other Choice-based tests in types module however the places testing choice were more integraion-y and less about the helper messages; so the test I added stands out like a lone wolf.
  • Nothing is really "changing" so I opted for not putting .. versionchanged:: anywhere
  • I didn't see any docs/ relevant to making custom choice types so I didn't any the top-level use-case there.
  • I did add a methoc doc though just for the sake of the checkbox, however the other helper fail fn doesn't have a method doc, so this stands out.

@AndreasBackx
Copy link
Collaborator

@antazoey thanks for getting back so quickly and my apologies for taking another week myself. I've wrapped it up and merged it, thank you for this fix!

@AndreasBackx AndreasBackx merged commit 4271fe2 into pallets:main Nov 3, 2024
11 of 12 checks passed
@antazoey antazoey deleted the feat/fail-method branch November 3, 2024 14:58
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.

Allow customizing fail message for invalid choice in click.types.Choice
2 participants