-
Notifications
You must be signed in to change notification settings - Fork 14
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
Force user to use bool
for delete_existing_job
#1437
Conversation
for more information, see https://pre-commit.ci
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.
I like fixing this problem, but I actually totally disagree with the attack. Let's rather go back to whatever __init__
is responsible and just add *
to force the subsequent values to be keyword-only.
That’s actually more or less what I suggested when the possibility to put multiple keywords was introduced. I cannot remember why it got rejected, but it got rejected at that time. Anyway wouldn’t it be a bit problematic concerning the backward compatibility? |
That's unfortunate, it sure seems like the right solution now...
🤦♂️ Ugh, yes. Ok, I'll review this as-is. Stupid backwards compatibility. |
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.
So the the caveat of our preceding conversation about how I wish we were in a different timeline, lgtm.
The only thing I would recommend is to make a custom error class so that when you're running the behaviour tests you can be 100% sure you're catching exactly the value error you're trying to catch and not accidentally passing because some other, unrelated value error is getting raised.
I constantly fall into the trap to set a job name like:
without realising I had to write more parentheses. As the first argument after the job name is
delete_existing_job
, this often erases the job, which just shouldn't happen for a small typo.