118 lines
4.4 KiB
Markdown
118 lines
4.4 KiB
Markdown
# PR#98: Rules for raster images
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
|
|
|
## Related Links
|
|
|
|
- [werkregie PR#55](https://git.javagil.de/mi/werkregie/pulls/55): YELLOW only because of `doc/images/dashboard.png`.
|
|
|
|
## Problem
|
|
|
|
A PNG file has no text diff, so neither Jev nor the model can judge it.
|
|
werkregie PR#55 added a screenshot of 177 KB and got two YELLOW reasons for it:
|
|
"changed files not judged by Jev" and "files the model cannot review".
|
|
A human then had to approve a PR that was otherwise GREEN.
|
|
|
|
A raster image cannot hold code that the review must see, but it can be too large for a repository,
|
|
or be another file, e.g. an archive, with an image extension.
|
|
|
|
## Non-Goals
|
|
|
|
- No rule for SVG: it is text, so Jev and the model see it already.
|
|
- No change for other binary files, e.g. `.jar`, `.zip` or `.pdf`: they stay YELLOW.
|
|
- No check of the image content.
|
|
|
|
## Specification
|
|
|
|
### Feature: a raster image is checked by its size and signature
|
|
|
|
#### Scenario#98.01: a small image
|
|
|
|
So that a screenshot in the documentation needs no human.
|
|
|
|
- **Given** a PR that adds a PNG file of 177 KB with a PNG signature
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the image gives only a hint with its size, and no YELLOW reason
|
|
|
|
##### Verified by
|
|
|
|
- [test_images.ImageFlowTest.test_a_small_image_gives_only_a_hint](../../tests/test_images.py)
|
|
- [test_images.ImageFlowTest.test_a_small_image_needs_no_judgment_by_jev](../../tests/test_images.py)
|
|
|
|
#### Scenario#98.02: an image above the size limit
|
|
|
|
So that a repository does not grow by large images unnoticed.
|
|
|
|
- **Given** a PR that adds a PNG file larger than `[images] max_bytes`
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the verdict is YELLOW, and the reason names the file and the limit
|
|
|
|
##### Verified by
|
|
|
|
- [test_images.ImageFlowTest.test_an_image_above_max_bytes_is_yellow](../../tests/test_images.py)
|
|
|
|
#### Scenario#98.03: images above the total limit
|
|
|
|
So that many small images do not bypass the limit per file.
|
|
|
|
- **Given** a PR whose images are each below `[images] max_bytes`, but larger than `[images] max_total_bytes` together
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the verdict is YELLOW, and the reason names the total and the limit
|
|
|
|
##### Verified by
|
|
|
|
- [test_images.ImageFlowTest.test_images_above_max_total_bytes_are_yellow](../../tests/test_images.py)
|
|
|
|
#### Scenario#98.04: a file with an image extension but another content
|
|
|
|
So that no archive or program enters the repository as an image without a human.
|
|
|
|
- **Given** a PR that adds a file `logo.png` whose first bytes are no PNG signature
|
|
- **When** werkrevisor reviews the PR
|
|
- **Then** the verdict is YELLOW, and the reason names the file
|
|
|
|
##### Verified by
|
|
|
|
- [test_images.ImageFlowTest.test_another_content_with_an_image_extension_is_yellow](../../tests/test_images.py)
|
|
|
|
#### Scenario#98.05: a repository may only lower the limits
|
|
|
|
So that a PR author cannot raise the limits through the repository config.
|
|
|
|
- **Given** a host config with the default limits
|
|
- **When** a `.werkrevisor.toml` sets `[images] max_bytes` above the host value
|
|
- **Then** the config is invalid
|
|
|
|
##### Verified by
|
|
|
|
- [test_images.ImageConfigTest.test_a_repository_must_not_raise_the_limits](../../tests/test_images.py)
|
|
|
|
## Solution
|
|
|
|
The new section `[images]` names the globs of the raster images and two size limits: 512 KB per image, 2 MB per PR.
|
|
Only PNG, JPEG, GIF, WebP and ICO count, whose first bytes werkrevisor knows; other `globs` cannot add a format.
|
|
A repository may replace the globs and only lower the limits.
|
|
|
|
`evaluate_rules` takes the images out of the files for Jev; an image outside the other categories counts as `harmless`.
|
|
`images.check` reads at most `max_bytes` plus one byte of each image at the head commit,
|
|
so a huge file does not fill the memory; `GiteaClient.get_raw_file` got the parameter `max_bytes` for it.
|
|
The checked images join the files that the model does not get, like the mechanical files,
|
|
so the coverage check does not miss them either.
|
|
A PR of images and mechanical files only skips Jev and the model.
|
|
|
|
The diagrams of the review name the images.
|
|
|
|
## Open Questions
|
|
|
|
None.
|
|
|
|
## Additional Changes
|
|
|
|
- `test_unreviewable_binary_is_yellow` uses a `.jar` file, since a PNG file is now an image.
|
|
|
|
## Follow-up PRs
|
|
|
|
None.
|