From 6459856aabedae167980e045121b9b33fabd1b4d Mon Sep 17 00:00:00 2001 From: Austin Schaefer Date: Sun, 20 Sep 2026 11:08:25 +0200 Subject: [PATCH] Group release sources into a source module with re-exports Move the ReleaseSource trait, GithubEndpoints and ForgejoEndpoints under src/source/, each in its own file (release_source.rs, github.rs, forgejo.rs). The submodules are private; mod.rs re-exports their types and holds for_package, so the rest of the crate imports from crate::source and never names a host's file. checker.rs keeps only version_from_tag. Co-Authored-By: Claude Sonnet 5 --- docs/ARCHITECTURE.md | 8 +++++--- docs/SPEC.md | 8 +++++--- src/checker.rs | 25 +++---------------------- src/fetcher.rs | 2 +- src/main.rs | 2 -- src/pipeline.rs | 7 +++---- src/{ => source}/forgejo.rs | 2 +- src/{ => source}/github.rs | 2 +- src/{source.rs => source/mod.rs} | 19 +++++++++++++------ src/source/release_source.rs | 20 ++++++++++++++++++++ 10 files changed, 52 insertions(+), 43 deletions(-) rename src/{ => source}/forgejo.rs (99%) rename src/{ => source}/github.rs (99%) rename src/{source.rs => source/mod.rs} (73%) create mode 100644 src/source/release_source.rs diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4440080..8e0a9b5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -35,8 +35,10 @@ practice rather than asserted from habit — see Further reading. `helpers/`) tends toward the opposite, and a single feature change ends up touching files scattered across every layer. **Rule**: modules are named after what they do in the pipeline - (`checker`, `fetcher`, `verifier`, `builder`, `sanity`, `publisher`, - `state`), not generic buckets. A new pipeline stage gets a new module + (`source`, `fetcher`, `verifier`, `builder`, `sanity`, `publisher`, + `state`), not generic buckets. (`source` is a directory module: the + `ReleaseSource` trait and one file per host, re-exported from its + `mod.rs` so the rest of the crate never names a host's file.) A new pipeline stage gets a new module named after the stage, not a method bolted onto an existing one. **Anti-example to keep watching for**: a `utils.rs` grab-bag. `hash.rs` could look like one but isn't — it exists for exactly one piece of @@ -107,7 +109,7 @@ practice rather than asserted from habit — see Further reading. directory, but the same information — why this way and not the obvious alternative — needs to live somewhere a future reader will actually see it: the doc comment on the thing itself. - **Example already here**: `github.rs`'s doc comment on + **Example already here**: `source/github.rs`'s doc comment on `GithubEndpoints::latest_release` explains why the newest Atom-feed entry isn't trusted outright (scaleway-cli's `-dbg1` tag has no real Release behind it) — the reasoning lives right next to the code it justifies, not in a diff --git a/docs/SPEC.md b/docs/SPEC.md index b3262bd..02edb5f 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -373,10 +373,12 @@ Open questions on the schema: likely reuses `nvchecker`'s logic/sources conceptually for non-GitHub sources eventually. For GitHub sources, prefers the `github-atom` feed (see Scaling > Check method) over unconditional REST polling. - *(Implemented for GitHub and Forgejo. `src/checker.rs` defines the + *(Implemented for GitHub and Forgejo. `src/source/` defines the `ReleaseSource` trait (latest release + API root); each host implements - it in its own module (`github.rs`, `forgejo.rs`), and `source.rs` picks - one per package, so adding a host doesn't touch existing ones. A trait + it in its own file (`github.rs`, `forgejo.rs`), and the module's + `for_package` picks one per package, so adding a host doesn't touch + existing ones. The rest of the crate imports the trait and hosts from + `crate::source`, which re-exports them. A trait rather than an enum match because there are now two real hosts with genuinely different logic. GitHub regex-matches the first `releases/tag/` link in the feed rather than doing a full XML diff --git a/src/checker.rs b/src/checker.rs index 33aa612..7d1b17c 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -1,25 +1,6 @@ -//! The check stage's contract: a `ReleaseSource` says what a package's -//! latest release is and where its releases API lives, and -//! `version_from_tag` turns the resulting tag into a version string. Each -//! hosting service implements the trait in its own module (`github`, -//! `forgejo`) so per-host logic doesn't accumulate here; -//! `source::for_package` picks the implementation for a package. - -use anyhow::Result; - -/// A hosting service a package's releases are published on. `Debug` so a -/// `Box` can sit in a `Result` that tests unwrap. -pub trait ReleaseSource: std::fmt::Debug { - /// The latest release tag for `repo`. - fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result; - - /// The releases API root, which `fetcher` and `verifier` build their - /// own paths under. GitHub and Forgejo both serve - /// `/repos/{owner}/{repo}/releases/tags/{tag}` there with the same - /// `assets[].{name, browser_download_url}` shape, which is why those - /// stages need only this and not the source itself. - fn api(&self) -> &str; -} +//! Turns a release tag into a version string. What the latest tag *is* +//! comes from a `ReleaseSource` (see `source`); this is the one piece of +//! the check stage that isn't host-specific. /// Strips a leading `v` from a release tag, e.g. `v2.62.0` -> `2.62.0`. /// diff --git a/src/fetcher.rs b/src/fetcher.rs index 3fbafaa..81f032d 100644 --- a/src/fetcher.rs +++ b/src/fetcher.rs @@ -1,6 +1,6 @@ //! Downloads a named release asset (from GitHub or Forgejo) to a local //! path. The only module that talks to the releases API for asset bytes — -//! `checker` only resolves version tags, never downloads. +//! `source` only resolves version tags, never downloads. use anyhow::{Context, Result}; use serde::Deserialize; diff --git a/src/main.rs b/src/main.rs index 3cc70d1..ac25251 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,8 +6,6 @@ mod builder; mod checker; mod config; mod fetcher; -mod forgejo; -mod github; mod hash; mod notifier; mod paths; diff --git a/src/pipeline.rs b/src/pipeline.rs index 5bcb61f..2723382 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -6,14 +6,14 @@ //! `main.rs`. use crate::builder; -use crate::checker::{self, ReleaseSource}; +use crate::checker; use crate::config::{self, Package}; use crate::fetcher::{self, DownloadedAsset}; use crate::notifier::{self, Event}; use crate::paths::Paths; use crate::publisher; use crate::sanity; -use crate::source; +use crate::source::{self, ReleaseSource}; use crate::state; use crate::verifier::{self, VerificationResult}; use anyhow::{Context, Result, bail}; @@ -353,8 +353,7 @@ fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &s #[cfg(test)] mod tests { use super::*; - use crate::forgejo::ForgejoEndpoints; - use crate::github::GithubEndpoints; + use crate::source::{ForgejoEndpoints, GithubEndpoints}; use crate::test_support::same_origin_package; #[test] diff --git a/src/forgejo.rs b/src/source/forgejo.rs similarity index 99% rename from src/forgejo.rs rename to src/source/forgejo.rs index af1ba00..6ff070a 100644 --- a/src/forgejo.rs +++ b/src/source/forgejo.rs @@ -1,7 +1,7 @@ //! A Forgejo (or Gitea) instance as a release source: its API root, and //! how to find a repo's latest release there. -use crate::checker::ReleaseSource; +use super::ReleaseSource; use anyhow::{Context, Result, bail}; use serde::Deserialize; diff --git a/src/github.rs b/src/source/github.rs similarity index 99% rename from src/github.rs rename to src/source/github.rs index 124349c..eac9803 100644 --- a/src/github.rs +++ b/src/source/github.rs @@ -1,7 +1,7 @@ //! GitHub as a release source: its endpoints, and how to find a repo's //! latest release there. -use crate::checker::ReleaseSource; +use super::ReleaseSource; use anyhow::{Result, bail}; use regex::Regex; diff --git a/src/source.rs b/src/source/mod.rs similarity index 73% rename from src/source.rs rename to src/source/mod.rs index 2d42a6d..615a0bc 100644 --- a/src/source.rs +++ b/src/source/mod.rs @@ -1,11 +1,18 @@ -//! Picks the `ReleaseSource` implementation for a package from its -//! configured `source`. The one place that knows which hosts exist; the -//! pipeline and the stages it calls only ever see the trait. +//! Where a package's releases are published: the `ReleaseSource` trait, +//! one file per host implementing it, and `for_package`, which picks the +//! implementation for a package from its configured `source`. The +//! submodules are private and re-exported here, so the rest of the crate +//! imports everything from `crate::source` and never a host's file. + +mod forgejo; +mod github; +mod release_source; + +pub use forgejo::ForgejoEndpoints; +pub use github::GithubEndpoints; +pub use release_source::ReleaseSource; -use crate::checker::ReleaseSource; use crate::config::{Package, Source}; -use crate::forgejo::ForgejoEndpoints; -use crate::github::GithubEndpoints; use anyhow::{Context, Result}; /// Defensive: errors only if a `forgejo-release` package has no diff --git a/src/source/release_source.rs b/src/source/release_source.rs new file mode 100644 index 0000000..2cde371 --- /dev/null +++ b/src/source/release_source.rs @@ -0,0 +1,20 @@ +//! The `ReleaseSource` trait: what the pipeline needs from a hosting +//! service a package's releases are published on. Each host implements it +//! in its own file next to this one, so per-host logic never accumulates +//! here. + +use anyhow::Result; + +/// `Debug` so a `Box` can sit in a `Result` that tests +/// unwrap. +pub trait ReleaseSource: std::fmt::Debug { + /// The latest release tag for `repo`. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result; + + /// The releases API root, which `fetcher` and `verifier` build their + /// own paths under. GitHub and Forgejo both serve + /// `/repos/{owner}/{repo}/releases/tags/{tag}` there with the same + /// `assets[].{name, browser_download_url}` shape, which is why those + /// stages need only this and not the source itself. + fn api(&self) -> &str; +}