Skip to content

Commit 1b094ba

Browse files
fix first release preflight flag
1 parent 110508e commit 1b094ba

7 files changed

Lines changed: 78 additions & 14 deletions

File tree

README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ cargo install --git https://github.com/Open-Tech-Foundation/release
6565

6666
| Gap | Impact |
6767
| --- | --- |
68-
| `--first-release` is not wired through the version flow yet. | First-release ergonomics are still stricter than the CLI help implies. |
6968
| `publish` lifecycle hooks run per adapter, not once per full command. | Polyglot repos may run global `pre_publish` / `post_publish` hooks more than intended. |
7069
| `snapshot` is experimental. | Multi-adapter semantics, generated notes, rollback expectations, and workflow polish need more hardening. |
7170
| Generated `release.yml` still needs stronger validation. | It is useful scaffolding, but complex monorepos may need hand edits. |

crates/cli/tests/e2e.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ fn version_then_publish_ships_exactly_the_computed_bumps() {
155155
git(root, &["add", "-A"]);
156156
git(root, &["commit", "-q", "-m", "init"]);
157157
git(root, &["branch", "-M", "main"]);
158+
git(root, &["tag", "@x/core@1.0.0"]);
158159
git(root, &["tag", "@x/sdk@1.0.0"]);
159160

160161
let remote = tempfile::tempdir().unwrap();

crates/cli/tests/version_flow.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ use anyhow::Result;
1212

1313
use otf_release_adapters::npm::{CommandOutput, CommandRunner, NpmAdapter};
1414
use otf_release_core::adapter::{Bump, Pkg};
15+
use otf_release_core::config::ReleaseConfig;
1516
use otf_release_core::forge::Forge;
1617
use otf_release_core::git::GitRepo;
1718
use otf_release_core::prompt::Prompt;
1819
use otf_release_core::version::{orchestrate, VersionOptions};
19-
use otf_release_core::config::ReleaseConfig;
2020

2121
/// Every `npm` invocation "succeeds" (the version flow only calls `update_lockfile`).
2222
struct OkRunner;
@@ -118,7 +118,7 @@ fn write_workspace(root: &Path) {
118118
);
119119
}
120120

121-
/// Init the repo on `main`, commit, and tag sdk so it isn't treated as a first release.
121+
/// Init the repo on `main`, commit, and tag publishable packages so they are not first releases.
122122
fn init_repo(root: &Path) {
123123
git(root, &["init", "-q"]);
124124
git(root, &["config", "user.email", "t@t"]);
@@ -127,6 +127,7 @@ fn init_repo(root: &Path) {
127127
git(root, &["add", "-A"]);
128128
git(root, &["commit", "-q", "-m", "init"]);
129129
git(root, &["branch", "-M", "main"]);
130+
git(root, &["tag", "@x/core@1.0.0"]);
130131
git(root, &["tag", "@x/sdk@1.0.0"]);
131132
}
132133

crates/core/src/preflight.rs

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,29 @@ pub struct Violation {
1919
pub message: String,
2020
}
2121

22+
/// Preflight behavior switches supplied by the caller.
23+
#[derive(Debug, Clone, Copy, Default)]
24+
pub struct CheckOptions {
25+
/// Permit publishable packages with no prior `name@x.y.z` tag.
26+
pub allow_first_release: bool,
27+
}
28+
2229
/// Run the gate. `selected` is the set of package names the user chose to bump (empty when
2330
/// preflight runs before the prompt). Returns every violation found; an empty vec means pass.
2431
pub fn check(
2532
repo: &dyn RepoState,
2633
packages: &[Pkg],
2734
selected: &[String],
35+
) -> Result<Vec<Violation>> {
36+
check_with_options(repo, packages, selected, CheckOptions::default())
37+
}
38+
39+
/// Run the gate with explicit behavior switches.
40+
pub fn check_with_options(
41+
repo: &dyn RepoState,
42+
packages: &[Pkg],
43+
selected: &[String],
44+
opts: CheckOptions,
2845
) -> Result<Vec<Violation>> {
2946
let mut violations = Vec::new();
3047

@@ -39,7 +56,10 @@ pub fn check(
3956
let pkg_dir = pkg.manifest_path.parent().unwrap_or_else(|| Path::new("."));
4057

4158
let violation = match repo.last_tag(&pkg.name)? {
42-
// First release: a publishable package with no prior tag must have notes.
59+
// First releases are explicit so accidentally untagged packages do not slip through.
60+
None if !opts.allow_first_release => {
61+
Some("first release requires --first-release".to_string())
62+
}
4363
None if empty => Some("first release but [Unreleased] is empty".to_string()),
4464
None => None,
4565
Some(tag) => {
@@ -143,9 +163,9 @@ mod tests {
143163
pkg(d, "core", true, Some(EMPTY)), // tag + commits + empty -> commits violation
144164
pkg(d, "utils", true, Some(WITH_NOTES)), // tag + commits + notes -> ok
145165
pkg(d, "sdk", true, Some(EMPTY)), // tag, no commits, empty, selected -> selected
146-
pkg(d, "new", true, Some(EMPTY)), // no tag, empty -> first-release violation
147-
pkg(d, "newgood", true, Some(WITH_NOTES)), // no tag, notes -> ok
148-
pkg(d, "miss", true, None), // no tag, missing changelog -> first-release
166+
pkg(d, "new", true, Some(EMPTY)), // no tag -> explicit first-release violation
167+
pkg(d, "newgood", true, Some(WITH_NOTES)), // no tag -> explicit first-release violation
168+
pkg(d, "miss", true, None), // no tag -> explicit first-release violation
149169
pkg(d, "app", false, Some(EMPTY)), // private -> skipped
150170
];
151171

@@ -166,7 +186,7 @@ mod tests {
166186
let violations = check(&repo, &packages, &selected).unwrap();
167187
let msgs = messages(&violations);
168188

169-
assert_eq!(violations.len(), 4, "got: {msgs:?}");
189+
assert_eq!(violations.len(), 5, "got: {msgs:?}");
170190
assert_eq!(
171191
msgs.get("core").unwrap(),
172192
"3 commit(s) since core@1.2.0 but [Unreleased] is empty"
@@ -177,17 +197,52 @@ mod tests {
177197
);
178198
assert_eq!(
179199
msgs.get("new").unwrap(),
180-
"first release but [Unreleased] is empty"
200+
"first release requires --first-release"
201+
);
202+
assert_eq!(
203+
msgs.get("newgood").unwrap(),
204+
"first release requires --first-release"
181205
);
182206
assert_eq!(
183207
msgs.get("miss").unwrap(),
184-
"first release but [Unreleased] is empty"
208+
"first release requires --first-release"
185209
);
186210
assert!(!msgs.contains_key("utils"));
187-
assert!(!msgs.contains_key("newgood"));
188211
assert!(!msgs.contains_key("app"));
189212
}
190213

214+
#[test]
215+
fn first_release_flag_allows_untagged_packages_but_still_requires_notes() {
216+
let tmp = tempfile::tempdir().unwrap();
217+
let d = tmp.path();
218+
let packages = vec![
219+
pkg(d, "new", true, Some(WITH_NOTES)),
220+
pkg(d, "miss", true, None),
221+
];
222+
let repo = FakeRepo {
223+
tags: HashMap::new(),
224+
counts: HashMap::new(),
225+
};
226+
227+
let violations = check_with_options(
228+
&repo,
229+
&packages,
230+
&[],
231+
CheckOptions {
232+
allow_first_release: true,
233+
},
234+
)
235+
.unwrap();
236+
let msgs = messages(&violations);
237+
238+
assert_eq!(violations.len(), 1, "got: {msgs:?}");
239+
assert_eq!(
240+
msgs.get("miss").unwrap(),
241+
"first release but [Unreleased] is empty"
242+
);
243+
assert!(!msgs.contains_key("new"));
244+
}
245+
191246
#[test]
192247
fn clean_repo_has_no_violations() {
193248
let tmp = tempfile::tempdir().unwrap();

crates/core/src/version.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,14 @@ pub fn orchestrate_many(
159159
.iter()
160160
.flat_map(|ctx| ctx.packages.iter().cloned())
161161
.collect();
162-
let violations = preflight::check(repo, &all_packages, &[])?;
162+
let violations = preflight::check_with_options(
163+
repo,
164+
&all_packages,
165+
&[],
166+
preflight::CheckOptions {
167+
allow_first_release: opts.first_release,
168+
},
169+
)?;
163170
if !violations.is_empty() {
164171
bail!("{}", preflight::format_violations(&violations));
165172
}

docs/commands/version.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ otf-release version [--dry-run] [--first-release]
99
| Flag | Effect |
1010
| --- | --- |
1111
| `--dry-run` | Compute and print the plan ([summary](#5-summary--confirm)), write nothing. |
12-
| `--first-release` | Reserved for first-release ergonomics; currently exposed by the CLI but not wired through the flow. |
12+
| `--first-release` | Permit publishable packages with no prior `name@x.y.z` tag. Curated mode still requires release notes for packages you want to release. |
1313

1414
Implemented in `crates/core/src/version.rs`.
1515

docs/preflight.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ don't falsely mark a package as changed.
2929
| --- | --- |
3030
| Commits since last tag (scoped to pkg path) **but** `[Unreleased]` empty/missing |**ABORT** |
3131
| Selected for a bump **but** `[Unreleased]` empty |**ABORT** |
32-
| No last tag **and** publishable (first release) | Require `[Unreleased]` (later: explicit `--first-release`) |
32+
| No last tag **and** publishable (first release) without `--first-release` |**ABORT** |
33+
| No last tag **and** publishable with `--first-release` | Require release notes in curated mode |
3334
| `[Unreleased]` present **with** commits | ✓ OK |
3435
| Commits in a **private** package | ✓ OK — no changelog demanded |
3536

0 commit comments

Comments
 (0)