fix(uninstall): scan devDependencies.apm so --dev installs can be removed (closes #1549)#1552
Open
danielmeppiel wants to merge 2 commits into
Open
fix(uninstall): scan devDependencies.apm so --dev installs can be removed (closes #1549)#1552danielmeppiel wants to merge 2 commits into
danielmeppiel wants to merge 2 commits into
Conversation
…oved apm install --dev <pkg> writes the entry under devDependencies.apm in apm.yml, but apm uninstall <pkg> only read dependencies.apm. The result was an unconditional 'not found in apm.yml' warning and the dev entry leaking forever. Uninstall now scans both sections and writes back to whichever section the package lived in. Closes #1549 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes apm uninstall so packages installed with apm install --dev can be discovered and removed from devDependencies.apm, addressing issue #1549.
Changes:
- Extends uninstall dependency scanning to include both production and development APM dependency sections.
- Updates manifest write-back logic to remove matched packages from the appropriate section.
- Adds regression tests for uninstalling, dry-running, and preserving unrelated dev dependencies.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/commands/uninstall/cli.py |
Adds devDependencies.apm handling during uninstall. |
tests/unit/test_uninstall_dev_dependencies.py |
Adds regression coverage for uninstalling dev dependencies. |
CHANGELOG.md |
Documents the uninstall fix under Unreleased. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
Comment on lines
+143
to
+146
| if package in dev_deps: | ||
| dev_deps.remove(package) | ||
| elif package in prod_deps: | ||
| prod_deps.remove(package) |
|
|
||
| ### Fixed | ||
|
|
||
| - `apm uninstall <pkg>` now scans `devDependencies.apm` in addition to `dependencies.apm`, so packages added with `apm install --dev <pkg>` can actually be removed. Previously dev-only entries reported "not found in apm.yml" and leaked forever. (closes #1549, #1552) -- by @aetos382 |
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.
Closes #1549.
Problem
apm install --dev <pkg>writes the package underdevDependencies.apminapm.yml, butapm uninstall <pkg>only reads/writesdependencies.apm. The result is that every dev-installed package reportsand the entry leaks forever. There is no way to remove a
--devinstall short of editingapm.ymlby hand.Fix shape
apm uninstallnow scans bothdependencies.apmanddevDependencies.apmwhen looking for matches, and writes the package out of whichever section it lives in. Symmetric to install:--devinstalls put the entry indevDependencies.apm; uninstall pulls it back out of the same section. EmptydevDependencieswrappers that we synthesised purely to satisfy the read are dropped before writing, so manifests for projects that never used--devstay byte-identical to today.Tests
New regression trap at
tests/unit/test_uninstall_dev_dependencies.py:test_uninstall_removes_package_from_dev_dependencies-- the headline regression: a package recorded underdevDependencies.apmis removed end-to-end.test_uninstall_dry_run_finds_dev_dependency----dry-runlocates the dev entry and leavesapm.ymluntouched.test_uninstall_preserves_unrelated_dev_dependency-- removing one dev dep does not touch other dev or prod entries.Mutation-break gate: deleting the new
devDependencieshandling makes all three tests fail with the originalnot found in apm.ymlsymptom; restoring the fix makes them pass. The fulltests/unit/ -k 'uninstall or dev_depend'suite (227 tests) andtests/integration/uninstall coverage (23 tests) stay green.How to test
Validation evidence
uv run --extra dev ruff check src/ tests/-- silentuv run --extra dev ruff format --check src/ tests/-- silentuv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/-- 10.00/10bash scripts/lint-auth-signals.sh-- cleanpytest tests/unit/ -k 'uninstall or dev_depend'-- 227 passedpytest tests/integration/test_uninstall_*.py tests/integration/test_wave5_e2e_coverage.py -k uninstall-- 23 passed, 2 skipped