bc1bb/backup-script
1
0
Fork 1
Code Issues 43 Pull requests 41 Projects Releases Packages Wiki Activity Actions

fix(discovery): drop unused include_images/include_volumes_outside_compose from discover (S1172) #62

Open
Claude wants to merge 2 commits from Claude/backup-script:fix/issues-32-33-S1172-docker-discover-unused-params into master
pull from: Claude/backup-script:fix/issues-32-33-S1172-docker-discover-unused-params
merge into: bc1bb:master
Conversation 0 Commits 2 Files changed 2 -4
Claude commented 2026-05-18 01:24:43 +02:00
First-time contributor
Copy link

Closes #32
Closes #33

include_images and include_volumes_outside_compose are declared on
gardien.discovery.docker.discover() but never read inside the body —
the function is a stub that just returns
DockerDiscovery(enabled=enabled). SonarQube flagged both parameters
as python:S1172 (unused function parameter).

This PR:

  1. Drops both parameters from the discover() signature in
    src/gardien/discovery/docker.py.
  2. Updates the one call site in
    Planner._build_docker_jobs (src/gardien/core/planner.py) to
    stop passing the corresponding kwargs.

The matching config flags on DockerCategoryConfig
(include_images: bool = False,
include_volumes_outside_compose: bool = True) are intentionally
preserved — they are still parsed from host.yaml and exposed on
the config model, so the caller can start using them again as soon as
the real docker volume ls / image-archive implementation lands. Only
the unused parameter plumbing on the function signature is being
removed.

Both issues are addressed in a single PR because they touch the same
def line and the same single call site — splitting them would
require two commits that each leave the function signature and its
caller out of sync. Treat as a paired fix.

Closes #32 Closes #33 `include_images` and `include_volumes_outside_compose` are declared on `gardien.discovery.docker.discover()` but never read inside the body — the function is a stub that just returns `DockerDiscovery(enabled=enabled)`. SonarQube flagged both parameters as `python:S1172` (unused function parameter). This PR: 1. Drops both parameters from the `discover()` signature in `src/gardien/discovery/docker.py`. 2. Updates the one call site in `Planner._build_docker_jobs` (`src/gardien/core/planner.py`) to stop passing the corresponding kwargs. The matching config flags on `DockerCategoryConfig` (`include_images: bool = False`, `include_volumes_outside_compose: bool = True`) are intentionally **preserved** — they are still parsed from `host.yaml` and exposed on the config model, so the caller can start using them again as soon as the real `docker volume ls` / image-archive implementation lands. Only the unused parameter plumbing on the function signature is being removed. Both issues are addressed in a single PR because they touch the same `def` line and the same single call site — splitting them would require two commits that each leave the function signature and its caller out of sync. Treat as a paired fix. 
The `discover()` stub never reads `include_images` or
`include_volumes_outside_compose` — it just returns
`DockerDiscovery(enabled=enabled)`. Drop both parameters from the
signature. The matching caller update in `planner._build_docker_jobs`
lands in a follow-up commit on the same branch.

The corresponding config flags on `DockerCategoryConfig` are preserved
— they remain parsed from `host.yaml` and stored on the model, ready
for the eventual real implementation. This PR only removes the unused
parameter plumbing on the function signature.

Closes #32
Closes #33
Follow-up to the prior commit on this branch: `docker_disco.discover()`
no longer accepts these parameters, so drop them from the call site in
`_build_docker_jobs`. The config flags themselves remain on
`DockerCategoryConfig` (parsed from `host.yaml`) so the caller is free
to start using them again once the discovery implementation needs them.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u fix/issues-32-33-S1172-docker-discover-unused-params:Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch Claude-fix/issues-32-33-S1172-docker-discover-unused-params

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git rebase master
git switch master
git merge --ff-only Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git rebase master
git switch master
git merge --no-ff Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch master
git merge --squash Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch master
git merge --ff-only Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git switch master
git merge Claude-fix/issues-32-33-S1172-docker-discover-unused-params
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
bc1bb/backup-script!62
No description provided.