Add --category to marketplace package add/set - #2626
Tech Guy (lukiod) wants to merge 2 commits into
Conversation
Marketplace manifest entries already support a category field (read, validated, and emitted by the existing schema/output code), but the CLI only exposed --tags, forcing manual apm.yml edits for categories. Adds --category to both commands, mirroring the existing --tags plumbing, with the same non-empty validation the schema already enforces on read.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for a category field on marketplace plugin entries, including CLI flags, YAML editing logic, validation, and unit tests.
Changes:
- Introduces
categoryoption toplugin addandplugin setcommands. - Adds
categoryto marketplace YAML add/update flows with validation. - Expands unit tests to cover
categorypersistence and blank-category errors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/unit/marketplace/test_yml_editor.py | Adds assertions and new tests for category handling in add/update flows. |
| src/apm_cli/marketplace/yml_editor.py | Adds _validate_category and wires category into add/update entry logic. |
| src/apm_cli/commands/marketplace/plugin/set.py | Adds --category flag and passes it through to update fields. |
| src/apm_cli/commands/marketplace/plugin/add.py | Adds --category flag and passes it through to entry creation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _validate_category(category: str) -> None: | ||
| """Validate *category* is a non-empty string.""" | ||
| if not isinstance(category, str) or not category.strip(): | ||
| raise MarketplaceYmlError("'category' must be a non-empty string") |
There was a problem hiding this comment.
Fixed in 5b89a07. _validate_category() now returns the stripped value and both call sites store what it returns, so --category " Productivity " writes Productivity. Two regression tests cover the add and update paths.
| if category is not None: | ||
| new_entry["category"] = category |
There was a problem hiding this comment.
Fixed in 5b89a07: the stripped value is what gets stored, at both call sites.
| if key == "subdir": | ||
| _validate_subdir(fields[key]) | ||
| elif key == "category": | ||
| _validate_category(fields[key]) | ||
| entry[key] = fields[key] |
There was a problem hiding this comment.
Fixed in 5b89a07: the stripped value is what gets stored, at both call sites.
|
@microsoft-github-policy-service agree |
Daniel Meppiel (danielmeppiel)
added
the
status/deferred
PR triage recommendationneeds-issue Recommendation only, not merge or scope approval. A responsible Linked issue#2625 -- open, labelled Proposed classification
Suggested next actionWait for a responsible human maintainer to record bounded scope, CODEOWNERS review requests currently include danielmeppiel and Suggested PR commentThank you for contributing this pull request. APM starts with an
issue, not an implementation
(https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md).
#2625 is the linked issue, but it is not labelled status/accepted.
A maintainer has withdrawn prior acceptance and labelled both the
issue and this PR status/deferred until bounded scope and a review
contact are recorded. Please keep discussion on #2625 rather than
opening a second issue, and please do not rebase or extend this
branch for APM while it is on hold.
This PR is labelled status/deferred. That is not acceptance,
assignment, or a merge invitation. Overlapping work is tracked on
#2637; any later review branch should be chosen with a maintainer
on the issue thread. |
Sergio Sisternes (sergio-sisternes-epam)
added
triage/recommended
_validate_category only checked that the stripped form was non-empty, so `--category " Productivity "` passed validation and the padding was written straight into the yml. The validator now returns the stripped value and both call sites store what it returns.
Closes #2625.
Marketplace manifest entries already support a
categoryfield — the schema validates it (yml_schema.py, "must be a non-empty string") and the output mappers already emit it (output_mappers.py), per the issue's own description. The CLI only exposed--tags, so getting a category intomarketplace.packages[]required a manualapm.ymledit.Adds
--category TEXTto bothapm marketplace package addandapm marketplace package set, mirroring the existing--tagsplumbing throughyml_editor.py'sadd_plugin_entry/update_plugin_entry, with the same non-empty validation the schema already enforces on read (so an invalid/blank value fails immediately with an actionable message rather than persisting bad data that only fails later oncheck).Verified end-to-end manually (
marketplace package add ... --category "Productivity"persists correctly;--category " "fails with'category' must be a non-empty string), plus new unit tests intest_yml_editor.pycovering both the happy path and the validation error foraddandset. Full marketplace test suite: 1511 passed.ruff checkclean on all changed files.Acceptance criteria from the issue:
--categorymarketplace.packages[]apm marketplace checkandapm packpreserve and emit category (already worked once present — pre-existing code, not touched here)