-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
refactor: Change no_answer
attribute
#3411
refactor: Change no_answer
attribute
#3411
Conversation
Label
answer
validationno_answer
attribute
Apparently, this modification is breaking fewer things/tests than I thought.
|
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.
Hey @anakin87! I had a look and I'm really happy about this one. It's crazy how much useless code you removed already 🤩
No need to go ahead and remove no_answer
for good: this is already totally worth merging as it is.
I don't see any dangerous change to be honest, especially because the value itself is still there in form of a property, so it's way less prone to break things. I also don't believe we need any tests for such a simple property.
I'll open a new issue with the details of what needs to be done to get rid of the property in the future. I'll tag you just to let you add any insights you might have, and if you want, to assist the next contributor that wants to test themselves with a big PR 😊
Hey great question! 😄 Let me ask @brandenchan or @TuanaCelik |
Hey, we've migrated the docs away from GitHub and are now a managed service called Readme. If you see something that can be changed, you can suggest it with the "Suggest Edits" button that is in the top right underneath the search bar |
Related Issues
Label
skipsanswer
validation ifno_answer=None
#3181Proposed Changes:
Make
answer
validation always run, when instantiating aLabel
,as suggested by @wochinge in #3181.
How did you test it?
Label
with invalidanswer
Checklist