Skip to content

Replace tracking method string constants with a TrackingMethod enum#237

Open
bbannier wants to merge 5 commits into
topic/bbannier/non-git-packagesfrom
topic/bbannier/tracking-method-enum
Open

Replace tracking method string constants with a TrackingMethod enum#237
bbannier wants to merge 5 commits into
topic/bbannier/non-git-packagesfrom
topic/bbannier/tracking-method-enum

Conversation

@bbannier

@bbannier bbannier commented Jul 17, 2026

Copy link
Copy Markdown
Member

The way we were storing the way a package came about with tracking method global strings was error prone. Any code using them was expected to figure out against which methods it should check. Since these were just unrelated global constants there was also no help from the type checker to make sure that handling was correct (forgot to check a variant?).

This PR cleans that up by introducing a proper Python enum, and then separating out Git-related tracking information from other concerns.

Warning

All code changes in this branch were generated with Claude Sonnet 4.6. I reviewed all changes and am accountable.

bbannier added 3 commits July 17, 2026 12:40
We introduce `TrackingMethod(str, Enum)` with members VERSION, BRANCH,
COMMIT, BUILTIN, and DIRECTORY. Using `str` as a mixin preserves JSON
round-tripping through the manifest without a custom encoder.

`PackageStatus.__init__` coerces plain strings to `TrackingMethod` so
existing manifests load transparently. The if/elif dispatch chains in
`upgrade()` and `_is_clone_outdated()` become `match` statements with
an exhaustive `case _: raise NotImplementedError` arm.
We introduce a `_GitTrackingMethod` type alias
(`Literal[VERSION, BRANCH, COMMIT]`) and a `TypeGuard`-returning
`_is_git_tracking_method` predicate.  `GitResolution`, `_pick_version`, and
`_is_clone_outdated` are updated to use this narrower type, so mypy can
verify that BUILTIN and DIRECTORY never reach the Git-only `match`
statements -- making `case _:` arms unnecessary.

We add `typing-extensions` as a runtime dependency for `assert_never` and
`TypeGuard`, both of which are available in stdlib only from Python 3.11
onward.
…tically"

This reverts commit 6cbbc43ce4599642910e8af6171b39018151e29b.
@bbannier bbannier self-assigned this Jul 17, 2026
bbannier added 2 commits July 17, 2026 14:06
BUILTIN and DIRECTORY were not tracking strategies -- they indicated the
source type of a package.  We remove them from the enum so `TrackingMethod`
describes only how a Git clone is followed (VERSION, BRANCH, COMMIT), and
non-Git packages express their state as `tracking_method=None`.

Older manifests that contain "builtin" or "directory" as string values are
handled in `PackageStatus.__init__`, which maps them to `None` for
backward compatibility.
With `TrackingMethod` covering only Git-backed methods, non-Git packages are
expressed as `tracking_method=None`.  The helper reduces to a `None` check
and adds no clarity over reading the condition directly.  We inline it at
both call sites and remove the function.
@bbannier
bbannier force-pushed the topic/bbannier/tracking-method-enum branch from 3ef2bff to dcba690 Compare July 17, 2026 12:08
@bbannier

Copy link
Copy Markdown
Member Author

Once the enum is in place this patch presents two possible ideas:

  1. Narrow tracking method types to enforce Git-only dispatch statically: add some annotations so mypy can typecheck things, and after reverting this
  2. Restrict TrackingMethod to Git-backed tracking strategies which properly models this

Of course we would only merge one approach.

I think I'd prefer 2) since it is conceptually simpler loosing anything in the remodel (I believe).

@bbannier
bbannier marked this pull request as ready for review July 17, 2026 12:25
@bbannier
bbannier requested a review from ckreibich July 17, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant