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

refactor: Fix boolean traps in signatures #287

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

pederhan
Copy link
Member

@pederhan pederhan commented Jan 21, 2025

This PR adds the Ruff rule FBT002, which detects positional boolean arguments in signatures that have a default value.

The affected function parameters are all changed to be keyword-only, so that we don't have hard-to-understand function invocations in the codebase wherein some positional bool decides the behavior of something, i.e.:

Authenticator.get_zabbix_url(True)

When it should be:

Authenticator.get_zabbix_url(prompt=True)

Adding this rule has highlighted that I am way too fond of this pattern, and I should be more mindful of how I write functions in the future. Does every function need some switch that enabled/disables some behavior? Looking at the codebase, probably not. These boolean toggles are rarely/never used.

FBT001 & FBT003

We cannot activate the FBT001 and FBT003 rules yet, since we use a lot of bool annotated positional args, as well as positional bool defaults for Typer options in commands. We need to migrate to the Annotated approach in order to use these without adding a bunch of # noqa comments all over the codebase (which is not worth it for essentially 0 gain).

@pederhan pederhan merged commit dc1650c into unioslo:master Jan 21, 2025
5 checks passed
@pederhan pederhan deleted the fix-boolean-traps branch January 21, 2025 09:02
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.

1 participant