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

Switch to keyword arguments for queue worker #775

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

evilstreak
Copy link
Member

The positional arguments are trouble waiting to happen. Switch over to keyword arguments which are less fragile, and can provide typing automatically.

We'll need to do at least one release with this before dropping positional arguments, to give folks a chance to update how they run Cog. We we do drop positional arguments we should make most of the keyword arguments required.

I've updated a couple of tests but left the rest, so we can be confident both positional and keyword arguments work. We can swap the rest when we remove positional args.

Copy link
Member

@bfirsh bfirsh left a comment

Choose a reason for hiding this comment

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

Oh my god yes.

The positional arguments are trouble waiting to happen. Switch over to
keyword arguments which are less fragile, and can provide typing
automatically.

We'll need to do at least one release with this before dropping
positional arguments, to give folks a chance to update how they run Cog.
We we do drop positional arguments we should make most of the keyword
arguments required.

I've updated a couple of tests but left the rest, so we can be confident
both positional and keyword arguments work. We can swap the rest when we
remove positional args.

Signed-off-by: Dominic Baggott <dominic.baggott@gmail.com>
@evilstreak evilstreak merged commit 4b54bab into main Nov 14, 2022
@evilstreak evilstreak deleted the switch-to-keyword-arguments branch November 14, 2022 12: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.

2 participants