Fallout-build / Fallout-build/Fallout

Centralise Fallout's own configuration constants in one place

Open
#584 0 comments 0 reactions 1 assignee Claimed by @ChrisonSimtian View on GitHub
enhancement
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

### Problem

Settings Fallout needs about itself — its own package ids, fallback versions, URLs, environment variable names, branch conventions — are spread across the codebase. Some live in `src/Fallout.Build.Shared/Constants.cs`. Others are a `private const` next to the single class that happens to use them. There is no rule for where a new one goes, so the default is a const at the call site, and the same value ends up written down more than once.

This came out of review on #581, where the tool package id was hardcoded in `build/Build.cs`. That one was solvable by reading `PackageId` off the csproj, but the same id is still a literal in `UpdateNotificationAttribute` and cannot be resolved that way (see [Notes](#notes)).

Found by scanning, not exhaustive:

| Setting | Where it is now |
|---|---|
| Tool package id (`Fallout.GlobalTool`) | Literal in `src/Fallout.Build/Execution/Extensions/UpdateNotificationAttribute.cs:38`, and `` in `Fallout.Cli.csproj` |
| Current + retired tool ids | `RewriteToolManifestStep.CurrentToolId` / `RetiredToolIds`, re-declared in `ResolveFalloutVersionStep.ToolPackageId` |
| Fallout version fallback (`10.3.49`) | `ResolveFalloutVersionStep.Fallback` |
| Pinned SDK version (`10.0.100`) | `BumpDotNetVersionStep.SdkVersion`, and `global.json` |
| `Fallout.Common` package id | `Constants.FalloutCommonPackageId`, and again as `ResolveFalloutVersionStep.PackageId` |
| nuget.org v3 index URL | `IPublish.cs:18`, `templates/Build.cs:32` |
| nuget.org flat-container URL | `NuGetVersionResolver.cs:18`, `ResolveFalloutVersionStep.cs:149` |
| `https://fallout.build` | `MigrateCommand.cs:129` (`Constants.FalloutDocsUrl` exists but is not used here) |
| Branch conventions (`main`, `release/`, `hotfix/`) | `GitRepositoryExtensions.cs` as literals; `build/Build.cs` as its own consts |
| Placeholder pack version (`9999.0.0`) | `Build.DefaultDeploymentVersion` |
| Fallback git remote (`origin`) | `GitRepository.FallbackRemoteName` |

### Outcome

One documented home for Fallout's own configuration. A contributor adding a setting has an obvious place to put it, and changing a value is a one-file edit. Values that can be derived (from a csproj, `global.json`, or the running assembly) are derived rather than written down at all.

### Acceptance criteria

- [ ] One place holds Fallout's own configuration constants, and it is reachable from every project that needs them.
- [ ] Every duplicated value in the table above resolves to a single declaration.
- [ ] A value that can be derived from an existing source of truth is derived, not copied. In particular the tool package id comes from `Fallout.Cli.csproj`'s `` rather than being typed a second time.
- [ ] Grouped by kind — package ids, versions and fallbacks, URLs, environment variable names, branch conventions — not one flat list.
- [ ] `docs/agents/conventions.md` says where a new setting goes, so the next one does not land as a call-site const.
- [ ] Existing behaviour is unchanged. This is a move, not a redesign.

### Open questions

- **Can this live in `Fallout.Core`?** Not as things stand. `Fallout.Core` targets `netstandard2.1;net10.0`. `Constants.cs` lives in `Fallout.Build.Shared`, which targets `netstandard2.0;net10.0`, and `Fallout.SourceGenerators` consumes `Fallout.Build.Shared.dll` directly (`Fallout.SourceGenerators.csproj:42`). Roslyn requires `netstandard2.0`, so moving these constants into `Fallout.Core` as it is today would cut the source generator off from them. Three ways out: keep the `netstandard2.0`-reachable project as the home and rename it; fix the `netstandard2.0` floor first; or split a `netstandard2.0` leaf the way `Fallout.Tooling.Abstractions` was split. Worth deciding before any code moves.
- **Constants or injectable configuration?** The title of this issue says constants, but #310 (FT-5, context-scope tool-path resolvers and process defaults) is heading toward per-run configuration. Some of the table above is genuinely fixed (env var names, URLs); some is arguably a default a consumer should be able to override (fallback versions, branch conventions). Deciding which is which is part of this work.
- `Constants.cs` mixes constants with path-building behaviour (`GetFalloutDirectory`, `GetParametersFileName`, and about ten more). Whether those move with it, or stay behind, is open.

### Notes

- `UpdateNotificationAttribute` cannot use the `Solution.Fallout_Cli.GetProperty("PackageId")` trick from #581. That works in `build/Build.cs` because it evaluates this repository's own csproj at build-authoring time. `UpdateNotificationAttribute` ships inside `Fallout.Common` and runs in a consumer's build, where no Fallout csproj exists. Deriving it there means flowing the id in at compile time, for example one MSBuild property in a shared `.props` feeding both `` and a generated constant.
- Related: #582 (package id is a consumer contract), #583 (`ProcessTasks` gaps), #310 (FT-5 process defaults).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.