Skip to content

Commit 9e09643

Browse files
authored
Merge pull request #657 from Fallout-build/upmerge/main-to-develop
Upmerge main into develop
2 parents 4a2df27 + bb7a127 commit 9e09643

32 files changed

Lines changed: 678 additions & 534 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
name: adding-a-migration-step
3+
description: How to make fallout-migrate handle a new rename, removal, or rewrite by extending an existing IMigrationStep, and when a genuinely new step is warranted. Trigger when asked to add a rename/rewrite rule to fallout-migrate, or to touch src/Fallout.Migrate/Migration.cs or its Steps.
4+
---
5+
6+
`fallout-migrate` runs a fixed, ordered list of `IMigrationStep` implementations,
7+
built in `src/Fallout.Migrate/Migration.cs`. A **step is one operation over one
8+
set of files**, and it owns the rewrite rules for those files.
9+
`RewriteCsprojsStep` holds the rules for `*.csproj`; `RewriteCsFilesStep` holds
10+
the rules for `*.cs`.
11+
12+
## Adding a new rename, removal, or rewrite
13+
14+
1. Find the step for that file type — `RewriteCsprojsStep` for `*.csproj`,
15+
`RewriteCsFilesStep` for `*.cs`, `RewriteBootstrapScriptsStep` for the
16+
bootstrap scripts.
17+
2. Add a `private static readonly Regex` field to that step, with a comment
18+
saying what moved and why.
19+
3. Apply it as another statement in the step's `Rewrite` method, incrementing
20+
`edits` per replacement.
21+
4. Add cases to that step's spec class in `tests/Fallout.Migrate.Specs`.
22+
23+
**Do not add a step per rename.** A new step means a second pass over the same
24+
files, so each file is written twice and the `Summary` edit count is inflated.
25+
Rule order also matters — a specific rule usually has to run before a general
26+
one — and inside one `Rewrite` method that order is plain statement order.
27+
Split across steps it becomes a hidden dependency between entries in
28+
`Migration.steps`.
29+
30+
## When a new step actually is warranted
31+
32+
Only for a new set of files or a genuinely different operation, such as
33+
renaming a directory or prompting the user. That's one new class implementing
34+
`IMigrationStep` plus one line in `Migration.steps`. If the new step depends on
35+
an earlier one, say so in the comment on that list entry, the way
36+
`ResolveFalloutVersionStep` does.
37+
38+
Keep `Steps/` free of helper classes. A rewriter that only serves one step
39+
belongs inside that step — see `aef6c073` (`CsprojRewriter`) and #528
40+
(`CodeRewriter`) for the shape.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
name: adding-a-tool-wrapper
3+
description: Recipe for adding or extending a CLI tool wrapper under src/Fallout.Common/Tools/<Tool>/<Tool>.json. Trigger when asked to add a new tool wrapper, add a command/argument to an existing one, or when a Tools/*.json file is being edited.
4+
---
5+
6+
Tool wrapper `.json` files are the source of truth; the `.cs` next to each one
7+
is generated — never hand-edit the generated file.
8+
9+
1. Find the closest existing tool under `src/Fallout.Common/Tools/<Tool>/<Tool>.json`
10+
and copy its shape.
11+
2. Cover a full command with all its arguments, not just the one option you need.
12+
3. Use formatting tags in `help` text:
13+
- `<c>` for inline code
14+
- `<a>` for links
15+
- `<ul>` / `<ol>` for lists
16+
- `<em>` for emphasized text
17+
- `<para/>` between paragraphs (not `<p>...</p>`)
18+
4. Don't write `secret: false` — it's the default.
19+
5. Don't write `default: xxx` — obsolete field, omit it.
20+
6. Run `./build.ps1 GenerateTools` to regenerate the `.cs` output.
21+
7. Commit the regenerated `.cs` alongside the `.json` spec in the same commit —
22+
`VerifyGeneratedTools` fails CI if they drift.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
name: creating-a-pr
3+
description: Steps for opening a pull request in the Fallout repo — picking the right base branch, creating it as a draft, applying the target/vCurrent-or-vNext and changelog-category labels, and handling breaking changes. Trigger whenever you are about to run `gh pr create`, decide a branch/base, write commit messages, or write a PR/issue description.
4+
---
5+
6+
Follow this exactly when opening a PR. Don't skip the labelling — it's easy to
7+
drop because it's just flags on the same `gh pr create` call.
8+
9+
## 0. Working from a fork?
10+
11+
Check `git remote -v`. If it shows both `origin` (your fork) and `upstream`
12+
(`Fallout-build/Fallout`), branch from `upstream/develop` — never
13+
`origin/develop`, which can be far behind and cause needless conflicts:
14+
15+
```bash
16+
git fetch upstream develop
17+
git switch -c <branch> upstream/develop
18+
git push -u origin <branch>
19+
gh pr create --repo Fallout-build/Fallout --draft ...
20+
```
21+
22+
Skip this for a plain single-remote clone.
23+
24+
## 1. Create as a draft
25+
26+
`gh pr create --draft` unless the user explicitly asked for ready-for-review.
27+
28+
## 2. Label at creation time, not as a follow-up
29+
30+
- **`target/vCurrent`** (default) or **`target/vNext`** (breaking changes — see
31+
below) — pass `--label target/vCurrent`.
32+
- **One changelog-category label** from [`.github/release.yml`](../../../.github/release.yml):
33+
`enhancement`, `bug`, `security`, `documentation`, `breaking-change`, or
34+
`skip-changelog` for housekeeping. Don't leave a PR uncategorized — it falls
35+
through to "Other Changes".
36+
37+
## 3. Breaking change? Do all of this too
38+
39+
A change is breaking if a commit uses the `!` suffix, has a `BREAKING CHANGE:`
40+
footer, or a reviewer would reasonably flag it (renamed/removed public API,
41+
package ID change, on-disk format change, CI/CD shape change consumers depend
42+
on) — except changes to `[Experimental]` surface, which carries no guarantee.
43+
44+
1. `--label target/vNext --label breaking-change` (use `breaking-change`
45+
instead of `enhancement`/`bug` as the changelog category).
46+
2. Open the PR body with a `⚠️ Breaking change` callout: name the affected
47+
surface and the consumer-side impact in one sentence.
48+
3. **Target `develop`**, never `release/vX.Y` or `main` — confirm the breaking
49+
surface sits behind `[Experimental("FALLOUT0xx")]` (see the
50+
`marking-experimental-apis` skill), or, if it can't be gated, on a
51+
short-lived branch off `develop` held for the next major. Don't bump
52+
`version.json`'s major — that happens once, at the cut.
53+
4. Spell out the migration path (one paragraph minimum) — what a consumer
54+
changes and what they run. There's no `CHANGELOG.md`; the `breaking-change`
55+
label is what carries this into the generated release notes.
56+
57+
If you only discover the breaking nature mid-review, apply all of this before
58+
requesting re-review.
59+
60+
## Writing the description
61+
62+
Follow the [plain-english skill](../plain-english/SKILL.md) for the terse,
63+
scannable shape (issues too) — lead with the point, bullets over prose, link
64+
don't recap.
65+
66+
## Full policy reference
67+
68+
[references/pr-creation-flow.md](references/pr-creation-flow.md) has the
69+
complete versioning-policy and milestone-labelling background behind the
70+
steps above, if you need the "why".
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# PR-creation flow — full policy background
2+
3+
The step-by-step procedure lives in the parent [SKILL.md](../SKILL.md). This
4+
file is the "why" behind it: the versioning policy a breaking-change PR must
5+
satisfy, and how milestones map to the `target/*` labels.
6+
7+
## Versioning policy
8+
9+
This project ships classic semver ([ADR-0009](../../../../docs/adr/0009-gitflow-and-semver-reversion.md)).
10+
The rule: **breaking changes wait for the next major**, and there's no fixed
11+
date for that — the project stays on `10.x` for as long as it can, and the
12+
eventual move to v11 will go through `fallout-migrate`.
13+
14+
There is **no `CHANGELOG.md`** — the file was retired. Release notes are
15+
generated from PR labels via [`.github/release.yml`](../../../../.github/release.yml).
16+
The PR description and its labels are now the lasting record of a change.
17+
18+
- A breaking change lands on **`develop`, behind `[Experimental("FALLOUT0xx")]`**
19+
(or, if that doesn't fit, on a short-lived branch off `develop` held until
20+
the cut). It does **not** bump `version.json`'s major mid-cycle.
21+
- **A `release/vX.Y` or `main` production line never takes a breaking change.**
22+
It only takes non-breaking work. The review before a production cut is the
23+
backstop that keeps an ungated breaking change off the production line.
24+
- Surface that isn't ready to commit to yet can ship behind
25+
`[Experimental("FALLOUT0xx")]` instead of being held back — see the
26+
`marking-experimental-apis` skill.
27+
28+
**Reviewer responsibility:** if a PR carries `!` (or a flagged breaking
29+
change), check that it targets `develop`, not a production branch. Check that
30+
the breaking surface is behind `[Experimental("FALLOUT0xx")]` (or on a topic
31+
branch, if it can't be gated). Check that the PR description has the
32+
`⚠️ Breaking change` callout with a migration path. Block the PR if any of
33+
that is missing.
34+
35+
## Milestones and version targeting
36+
37+
Milestones are **theme-based** (e.g. "Plugin Architecture Foundation &
38+
Rebrand Completion", "Public Plugin SDK", "Continuous Delivery Vision") and
39+
carry across releases; version targeting uses **evergreen `target/vCurrent` /
40+
`target/vNext`** labels — `target/vCurrent` is the current release line,
41+
`target/vNext` is the next major. A breaking change is held for the next
42+
major, so its PR carries `target/vNext`. See [docs/roadmap.md](../../../../docs/roadmap.md)
43+
for the current milestones.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
name: cutting-a-release
3+
description: Procedure for tagging, publishing, promoting, or cutting a new release/vX.Y branch, hotfixing a production line, or cutting the eventual v11 major. Trigger on requests to cut a release, publish to nuget.org, tag a version, hotfix main or support/v10, or promote a release branch to GA.
4+
---
5+
6+
The maintainer runbook is [docs/branching-and-release.md](../../../docs/branching-and-release.md)
7+
— read it and follow it exactly; this file is a quick index into it plus the
8+
gotchas worth knowing before you start.
9+
10+
## Which section of the runbook you need
11+
12+
| You're asked to... | Read |
13+
| --- | --- |
14+
| Cut a new `release/vX.Y` and tag `-rc.N` builds | [Cutting a release](../../../docs/branching-and-release.md#cutting-a-release) |
15+
| Publish a GA tag to nuget.org | [Publishing to nuget.org](../../../docs/branching-and-release.md#publishing-to-nugetorg) |
16+
| Ship an emergency fix to `main` or `support/v10` | [Hotfixing production](../../../docs/branching-and-release.md#hotfixing-production) |
17+
| Cut the eventual v11 | [Cutting v11](../../../docs/branching-and-release.md#cutting-v11-when-it-eventually-happens) |
18+
| Retire an old `support/*` line | [Deprecating a support/* line](../../../docs/branching-and-release.md#deprecating-a-support-line) |
19+
| First-ever publish of a new `Fallout.X` package | [First-publish gotcha](../../../docs/branching-and-release.md#adding-a-new-falloutx-package--first-publish-gotcha) |
20+
21+
## Gotchas worth knowing up front
22+
23+
- **`--ref` on `workflow_dispatch` must be the production branch**, not the
24+
default branch — it runs *that ref's copy* of the workflow.
25+
- **`--notes-start-tag` must be the last GA tag**, not the previous rc, or the
26+
generated notes only cover since the last rc.
27+
- **A green pipeline run is not proof anything published** — every publish job
28+
is conditional. Verify job conclusions and the nuget.org index after a
29+
release (commands in the runbook).
30+
- **nuget.org is always opt-in** via `workflow_dispatch -f publish-to-nugetorg=true`;
31+
a tag push alone only reaches GitHub Packages + GitHub Releases.
32+
- Never bypass a hotfix through direct pushes — branch protection blocks it
33+
everywhere; even a one-commit cherry-pick goes through a PR.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
name: editing-ci-workflows
3+
description: Invariants to preserve when touching .github/workflows/**, build/Build.CI.GitHubActions.cs, or anything about which branches/tags trigger CI or publish packages. Trigger before editing a workflow YAML file, the CI generator source, or branch trigger lists.
4+
---
5+
6+
Read [references/ci-invariants.md](references/ci-invariants.md) in full before
7+
changing a workflow — most mistakes here are "looks fine, quietly breaks a
8+
gating assumption." The short version:
9+
10+
- **`build.yml` and `build-cross-platform.yml` are generated** from
11+
`build/Build.CI.GitHubActions.cs` — edit the attributes/constants there and
12+
regenerate (`./build.sh`), never hand-edit those two `.yml` files.
13+
`build-skip.yml`, `publish-packages-preview.yml`, and
14+
`publish-packages-release.yml` are hand-written; those you do edit directly.
15+
- **Feature branches run zero CI** until a PR targets a long-lived branch
16+
(`develop`/`main`/`release/*`/`support/*`). Never add a working-branch
17+
pattern (`feature/*`, `bugfix/*`, …) to a push/PR trigger.
18+
- **Cross-platform (`windows`/`macos`) is release-intent-gated** — PRs into
19+
`main`/`release/*`/`support/*`, or a `v*` tag push. Never add `develop` to
20+
its push triggers.
21+
- **`concurrency: cancel-in-progress` everywhere except `publish-packages-release.yml`**
22+
— never cancel a publish mid-flight.
23+
- **Every publishing lane runs `Test` before it publishes**, as a single
24+
`dotnet fallout Test Pack` invocation, not split steps — splitting
25+
double-compiles.
26+
- Don't add `submodules: recursive` to a checkout step — there are no
27+
submodules in this repo.
28+
29+
See also [docs/architecture.md](../../../docs/architecture.md#ci-layout) for
30+
the current workflow-to-trigger table, and
31+
[docs/branching-and-release.md](../../../docs/branching-and-release.md) for
32+
the branch-protection rulesets these triggers back onto.
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# CI pipeline & trigger invariants
2+
3+
Shaped by [milestone #18](https://github.com/Fallout-build/Fallout/milestone/18)
4+
and the branch model in [ADR-0009](../../../../docs/adr/0009-gitflow-and-semver-reversion.md)
5+
(carrying forward [ADR-0008](../../../../docs/adr/0008-collapse-experimental-into-main.md),
6+
which collapsed `experimental` into the integration trunk).
7+
8+
- **Feature branches run zero CI until a PR is opened.** Push triggers list
9+
**only** long-lived branches; nothing fires on `feature/*`, `bugfix/*`, etc.
10+
until they're PR'd against `develop`/`main`/`release/*`/`support/*`. Do
11+
**not** add a working-branch pattern to any `OnPush*`/`branches:` trigger.
12+
- **The Linux PR gate (job `ubuntu-latest`, from `build.yml`) is the only
13+
required check** — runs on PRs to the long-lived branches. (Branch
14+
protection keys on the job name, not the workflow file.)
15+
- **A push to `develop` publishes `-preview`** to GitHub Packages
16+
(`publish-packages-preview.yml`). It's the only continuous publisher —
17+
there is still no `experimental.yml`.
18+
- **Cross-platform `windows`/`macos` only run on release intent** — one
19+
`build-cross-platform.yml` workflow (a job per OS), firing on a PR into
20+
`main`/`release/*`/`support/*`, or a `v*` tag push. They do **not** run on
21+
`develop` pushes. `develop` relies on the Linux gate instead.
22+
- **`concurrency: cancel-in-progress` on every build workflow except
23+
`publish-packages-release.yml`** — never cancel a publish mid-flight.
24+
- **Canonical CI-ignore paths:** `docs/**`, `.assets/**`, `**/*.md` — applied
25+
to every PR/push trigger.
26+
- The `build.yml` (Linux gate) and `build-cross-platform.yml` (macOS+Windows)
27+
workflows are **generated** from `build/Build.CI.GitHubActions.cs` — edit
28+
the attributes + constants there and regenerate (`./build.sh`), never
29+
hand-edit the `.yml`. `build-skip.yml`, `publish-packages-preview.yml`, and
30+
`publish-packages-release.yml` are hand-written.
31+
- **Every publishing lane runs `Test` before it publishes** (#324).
32+
`publish-packages-preview.yml` and `publish-packages-release.yml` both run
33+
a single `dotnet fallout Test Pack` invocation — NUKE executes it as
34+
discrete internal stages (Restore → Compile → Test → Pack) and fails at the
35+
breaking stage, so a test failure stops the job before the push step. Don't
36+
split a lane into separate `dotnet fallout Compile`/`Test`/`Pack` steps —
37+
each invocation re-runs the dependency graph (double-compile); the single
38+
invocation *is* the staged build.
39+
- **Caching** (#328): every workflow caches `~/.nuget/packages` +
40+
`.fallout/temp`, keyed on `global.json` + `**/*.csproj` +
41+
`Directory.Packages.props` (the dependency-affecting set), with a
42+
`restore-keys:` prefix fallback for partial restores. There is no
43+
`packages.lock.json` to add to the key, and build outputs (`bin`/`obj`) are
44+
deliberately **not** cached (stale-artifact correctness risk).
45+
- Don't add `submodules: recursive` to a checkout — there are no submodules
46+
(no `.gitmodules`); it's a dead init step.
47+
- Don't add `develop` (or any working-branch pattern) to the **push** triggers
48+
of the cross-platform workflows — they're release-intent-gated on purpose
49+
(milestone #18 / #318 / #326).
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
name: marking-experimental-apis
3+
description: How to mark a public API as not-yet-stable with [Experimental("FALLOUT0xx")], or deprecate one with [Obsolete(..., DiagnosticId = "FALLOUTOBS0xx")], including diagnostic-ID allocation. Trigger when adding public API that isn't ready for a stability guarantee, or removing/replacing/deprecating an existing public API.
4+
---
5+
6+
Fallout has two attributes for public-surface churn, each with its own
7+
diagnostic-ID sequence and registry. Never mix the two sequences.
8+
9+
## `[Experimental]` — opt-in unstable surface
10+
11+
Use [`System.Diagnostics.CodeAnalysis.ExperimentalAttribute`](https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.experimentalattribute)
12+
(ships in the .NET 8+ BCL, no package reference needed) for public API that
13+
isn't ready to commit to a stability guarantee:
14+
15+
```csharp
16+
using System.Diagnostics.CodeAnalysis;
17+
18+
[Experimental("FALLOUT001")]
19+
public sealed class NewPluginHost { /* ... */ }
20+
```
21+
22+
- **Allocate the next `FALLOUT0xx` ID sequentially, never reused.** Register it
23+
in [docs/experimental-apis.md](../../../docs/experimental-apis.md) in the
24+
same PR.
25+
- **`ExperimentalAttribute` is error-by-default.** A consumer must explicitly
26+
suppress the exact ID (`#pragma warning disable FALLOUT001` or `<NoWarn>`) to
27+
use the API — that's the opt-in.
28+
- **Promoting to stable = deleting the attribute.** No cross-branch dance —
29+
the feature already rode the `develop` preview lane. Adding or removing
30+
`[Experimental]` is **not** a breaking change.
31+
- **On a `release/vX.Y` / `main` production line, any risky-but-shipped public
32+
surface must wear it.** That's what keeps the production line trustworthy
33+
while it still carries new work — there's no separate `experimental` branch.
34+
- **Don't apply it speculatively.** Marking an API that's already used
35+
internally breaks the build everywhere it's referenced — suppress every
36+
internal usage in the same change.
37+
38+
## `[Obsolete]` — deprecating a stable API
39+
40+
Use [`System.ObsoleteAttribute`](https://learn.microsoft.com/dotnet/api/system.obsoleteattribute)
41+
with a `DiagnosticId` (ships in the .NET 5+ BCL) when a stable public API is on
42+
its way out:
43+
44+
```csharp
45+
[Obsolete(
46+
"Use [GitHubActionsInputAttribute] instead. Removed in v11.",
47+
DiagnosticId = "FALLOUTOBS001",
48+
UrlFormat = "https://github.com/Fallout-build/Fallout/blob/main/docs/obsolete_apis.md")]
49+
public string[] OnWorkflowDispatchOptionalInputs { get; set; } = new string[0];
50+
```
51+
52+
- **Allocate the next `FALLOUTOBS0xx` ID sequentially, never reused** — a
53+
separate sequence from `FALLOUT0xx` above. Register it in
54+
[docs/obsolete_apis.md](../../../docs/obsolete_apis.md) in the same PR.
55+
- **Always set `DiagnosticId`.** Without one the compiler reports the generic
56+
`CS0618`, so a `TreatWarningsAsErrors` consumer can only fix every usage at
57+
once or blanket-suppress every deprecation. A per-deprecation ID lets them
58+
suppress just this one while they migrate.
59+
- **Adding `[Obsolete]` is not a breaking change** — it's warning-level, so
60+
existing code keeps compiling. The *removal* is the break, and it waits for
61+
the next major. State the removal target in the message (e.g. `Removed in v11.`).
62+
- **Keep the deprecated surface functional.** Prefer bridging the old member to
63+
the new one over leaving it inert, and suppress the internal bridge usage
64+
with `#pragma warning disable` scoped to the exact ID.
65+
66+
See [AGENTS.md rule 2](../../../AGENTS.md) for how these fit the
67+
backwards-compatibility policy, and the `creating-a-pr` skill for how a
68+
breaking change (as opposed to marking something experimental or obsolete)
69+
needs to be labelled and targeted.

0 commit comments

Comments
 (0)