Repository navigation
Conversation
This bumps ruff from 0.15.20 to 0.17.2, Biome from 1.9.4 to 2.6.3 and djhtml to 3.0.11. ruff and djhtml are now also pinned in the `dev` extra, so that a local `pip install -e '.[dev]'` gets the same versions that CI uses. Before, whatever ruff happened to be installed locally could disagree with CI. `biome.json` was converted to the Biome 2 format with `biome migrate`, and Biome 2 formats multi-line CSS custom properties slightly differently. ruff 0.17 enables a much larger set of rules by default. The violations of those new rules are fixed in the following commits, one commit per rule.
UTF-8 is the default source encoding in Python 3, so these lines do nothing (ruff UP009).
These `__future__` features (`print_function`, `unicode_literals`, ...) are always enabled in Python 3, so the imports do nothing (ruff UP010).
In Python 3 every class inherits from `object` already (ruff UP004).
The Python 3 form `super()` is equivalent to `super(Class, self)` and does not need to be updated when a class is renamed (ruff UP008).
`datetime.UTC` is the shorter alias for `datetime.timezone.utc` that exists since Python 3.11 (ruff UP017).
Parentheses around a single expression in a call like `print((x))` are redundant (ruff UP034).
`getattr(obj, "name")` without a default is just a more verbose `obj.name` (ruff B009).
This is the more direct way to write the same import (ruff PLR0402).
f-strings are easier to read because the values are next to where they end up in the string (ruff UP032).
f-strings are easier to read because the values are next to where they end up in the string (ruff UP031). Most of this was converted automatically by ruff, but a few strings that index into a dict, which ruff would only convert to `str.format()`, were converted by hand.
ruff UP030 flags `"{0} {1}".format(a, b)`, because the indices are redundant. These were converted by hand instead of with ruff's fix, because that fix turned `"http{0}://{1}:{2}{3}"` into `"http{}://{}:{}{3}"`, which mixes automatic and manual numbering and raises a `ValueError` at runtime. The archive URLs are now f-strings, and the `WHERE` clause is built with plain concatenation.
This avoids building an intermediate list (ruff C403).
This avoids building an intermediate list of tuples (ruff C404).
`{a, b}` is shorter and avoids building an intermediate list (ruff C405).
A `{...}` literal is shorter and faster than calling `dict()` with keyword arguments (ruff C408).
This stops at the first match instead of building the whole list (ruff RUF015).
This makes it explicit that the value is intentionally ignored (ruff RUF059).
Replaces `if cond: return True; return False` with `return cond` (ruff SIM103).
A mutable default like `[]` is shared between all calls of the function, so changing it in one call affects later calls (ruff B006).
ruff RUF012 flags mutable class attributes, because a list or dict defined on a class is shared by all instances, and mutating it through one instance changes it for all of them. Annotating them as `ClassVar` makes explicit that they are meant to be class level settings, like Django's `Meta.ordering` or a form's `field_order`. I annotated them instead of converting the lists to tuples, because changing `choices` from a list to a tuple could make Django think the field changed and generate a new migration. Migrations are excluded from this rule, because `makemigrations` generates `dependencies` and `operations` as plain class attributes, and annotating every new migration by hand would be busywork.
ruff LOG015 flags calls like `logging.debug()`, which log through the root logger. A logger named after the module makes it clear where a log line came from. The output is unchanged, because the named logger propagates to the root logger that `logging.basicConfig()` configures.
ruff BLE001 flags `except Exception`, because it also hides programming errors such as a `TypeError` in our own code. `_archivesAPI()` now only turns `requests` errors (connection errors, timeouts and invalid JSON) into `Http503`, and raises `Http503` directly for an unexpected status code instead of a bare `Exception` (which ruff TRY002 also flags). `NewPatchForm` then only needs to catch `Http503`. Any other error now shows up as a regular server error, instead of a "Error in API call to validate thread" message that hides what went wrong.
ruff TRY002 flags raising a bare `Exception`. This one was dead code: the URL pattern for `close` only matches `reject`, `withdrawn`, `feedback` and `committed`, so `status` can never be `"next"`. Moving to the next commitfest has been handled by the `transition/` endpoint for a long time.
ruff PLW1510 wants `check` to be explicit, so it is clear whether a failing command should raise. Here it should not: stopping the dev server with Ctrl-C makes uwsgi exit non-zero, and a `CalledProcessError` traceback on top of that would just be noise.
ruff RUF100 flags `noqa` comments for rules that are not enabled. E402 and F403 are not in ruff's rule set, so these comments did nothing.
`pip install .` leaves a `build/` directory with a copy of the code behind. Because it was not ignored, `make lint` (ruff respects `.gitignore`) also linted that stale copy and reported errors that do not exist in the actual code.
Biome flags top-level functions that are never called from JavaScript, but most functions in `commitfest.js` are called from `onclick` attributes in the templates, which Biome cannot see. Those now have a `biome-ignore` comment saying so, instead of being deleted by its fix. `verify_next()` really was unused, because moving a patch to the next commitfest no longer asks for confirmation, so it is removed. The results of `new bootstrap.Collapse()` and `new bootstrap.Tooltip()` were assigned to variables that were never read, which are now dropped.
Callbacks whose only parameter was unused now take no parameters. Where an unused parameter comes before one that is used, like the index in `$.each(data, (i, m) => ...)`, it is prefixed with an underscore, which is how Biome expects intentionally unused parameters to be marked.
Biome useParseIntRadix wants the radix to be explicit, because without one a string with a `0x` prefix is parsed as hexadecimal.
Biome flags `!important`, because it makes styles hard to override later. Most of ours were not needed:
The `text-decoration: none` rules for badge links already win from Bootstrap's `a { text-decoration: underline }` on specificity alone.
The selectize scroll hint used `!important` only to beat the inline `border-top` and `background-color` that the JavaScript adds to the element. So those inline styles never had any effect, and both they and the `!important` are now removed.
The navbar `gap`, `align-items` and `width` rules do need `!important`, because they override Bootstrap's `gap-2` and `align-items-center` utility classes (which use `!important` themselves) and an inline `width`. These now have a `biome-ignore` comment explaining that.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates ruff from 0.15.20 to 0.17.2, Biome from 1.9.4 to 2.6.3 and djhtml to 3.0.11. ruff and djhtml are now also pinned in the
devextra, so that a local install uses the same versions as CI.ruff 0.17 and Biome 2 enable a lot more rules by default. Their violations are fixed in separate commits, one per rule, so that it is easy to drop any rule that we do not want to adopt.
A few of these needed more than the automatic fix:
ruff's own fix for UP030 turned
"http{0}://{1}:{2}{3}"into"http{}://{}:{}{3}", which raises aValueErrorat runtime, so those URLs were rewritten as f-strings by hand.For RUF012 the mutable class attributes are annotated with
ClassVarinstead of being converted to tuples, because tuplechoicescould make Django generate a new migration. Migrations are excluded from this rule, sincemakemigrationsgenerates plain class attributes.For BLE001
_archivesAPI()now only catchesrequestserrors. Unexpected errors used to be reported as "Error in API call to validate thread", and now show up as a regular server error.Most functions that Biome reported as unused are called from
onclickattributes in the templates, so they got abiome-ignorecomment instead of being deleted. Onlyverify_next()was really unused.Most
!importantstyles were not needed and are removed. The navbar ones override Bootstrap's own!importantutility classes, so they stay, with a comment explaining why.Finally
build/is now ignored, because the stale copy of the code thatpip install .leaves there mademake lintreport errors that do not exist in the actual code.