refactor: clean up what #1933 left behind - #1936
Merged
Merged
Conversation
After #1933, some comments still named what it removed or said the wrong thing about what stayed: - `stream_events_with_options` repeated half a sentence of its own summary. - `RunOptions` and `CommitOptions` pointed at `with_*` builders that no longer exist. - `list_tests.rs` described `Engine::list_volumes`; the kept entry point is `list_volumes_with_display`. - `ByteCounter` said its raw counter was for the libpod client's PUT helper, and that the upload-side producer was `put_archive_verified`. The counter goes to the upload body in `pack.rs`, and the wrapper that advances it is `receiver_body_with_counter`. - One test comment said extends resolution needs `parse_file`. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
- `CpOptions::with_index`: the command line sets the index through `CpOptions::new`, and its only other caller was a test that went with #1933. - `Table`'s per-row key: `push_keyed` was the only thing that set one, so every row carried `None` and `format_row_keyed` always fell back to the identity cell's own text. `format_row` now does that directly. - `build_vars_with_env_files_inner`'s `strict` flag: its only caller passes `true` since the lenient wrapper was removed, so the lenient branches could not run. The `.env` fallback keeps its lenient behaviour; it goes through `build_vars`, not this function. Nothing a user sees changes: the non-live suite passes unchanged (2960 with the tests of the next commit). Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
- `an_unprefixed_label_is_keyed_on_itself`: a label without the project prefix gets the colour of the service of that name, now compared on `identity_slot` and `service_slot`. Keying the unprefixed label on anything else fails it. - `set_services_makes_registered_names_distinct` also checks that the three distinct slots render as three distinct colours through `slot_to_style`; collapsing the wide palette to one colour fails it. - `a_board_with_every_row_finished_tallies_all_of_them`: finishing every seeded row tallies (3, 3). Capping the done count fails it. - `check_status_falls_back_to_raw_body_on_non_json` also checks the body is kept verbatim, not only contained; adding one character to it fails the test. `slot_to_style` gets back the reason its palette choice is a parameter: the real probe is process-cached, so a test that flipped it would depend on scheduling. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added
type:refactor
Jaro-c
enabled auto-merge (squash)
September 26, 2026 15:46
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
1 participant
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.
A second review of #1933 after it merged found fourteen things it left behind. None changes what a user sees; they are comments, tests and code that lost their last caller. I checked each one against the code before changing it.
Comments (first commit):
stream_events_with_optionsrepeated half a sentence of its own summary (my mistake when I restored its# Errorssection);RunOptionsandCommitOptionspointed atwith_*builders that are gone;list_tests.rsdescribedEngine::list_volumes;ByteCountersaid its raw counter was for a libpod PUT helper that no longer exists and named the wrong producer (put_archive_verifiedinstead ofreceiver_body_with_counter); a parse test comment namedparse_file.Code with no caller (second commit):
CpOptions::with_index;Table's per-row key, which onlypush_keyedever set, so every row carriedNone; and thestrictflag ofbuild_vars_with_env_files_inner, whose only caller passestrue, so its lenient branches could not run. The.envfallback keeps its lenient behaviour; it goes throughbuild_vars.Tests (third commit), for what the tests #1933 deleted still covered:
identity_slotagainstservice_slot); keying it on anything else fails it;Checks: fmt, clippy
-D warnings, doc-D warnings, the non-live suite (2960), thetest-helpersbuild, and the full live suite on Podman 5.7.0: 238 of 238.Refs #1925
Signed-off-by: Jaro-c 75870284+Jaro-c@users.noreply.github.com