Move the constant values into Fallout.Core - #586
Draft
ChrisonSimtian wants to merge 4 commits into
Draft
Conversation
5 tasks
Core is the innermost project, so it is where Fallout's own fixed values belong (Fallout-build#584). Everything now reads them from there. Split Constants.cs on the line between data and behaviour: - Fallout.Core/Constants.cs — the ~31 const values: directory and file names, parameter names, environment variable keys, package ids, project URLs. Adds FalloutGlobalToolPackageId, which UpdateNotificationAttribute previously carried as a literal. - Fallout.Build.Shared/FalloutPaths.cs — the ~19 helpers that derive a path or read the file system. These cannot follow: they need AbsolutePath, EnvironmentInfo and the file system, and Core references nothing. Fallout.Build.Shared now references Core, so the values have one home and the helpers consume them from it. Core gains netstandard2.0 for this. Fallout.SourceGenerators (Roslyn) and Fallout.MSBuildTasks (net472) both consume these values and cannot load a netstandard2.1 assembly. Core references nothing and uses no 2.1-only API, so the 2.0 build is the same source; netstandard2.1 stays so 2.1+ consumers keep the richer asset and future Core code can use 2.1 APIs behind an #if. The reason and the exit condition are in a comment on the csproj. Call sites: helper uses moved to FalloutPaths, either qualified or via an added `using static`. No behaviour change — every value and every helper body is identical to before. Verified: solution builds clean, Core emits all three assets, full test suite passes. Three GitRepositoryWorktreeSpecs failures are pre-existing on this checkout (its `origin` is the fork, so the parsed identifier is ChrisonSimtian/Fallout rather than Fallout-build/Fallout) and reproduce unchanged at upstream/main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment named Fallout.SourceGenerators and Fallout.MSBuildTasks as the consumers forcing netstandard2.0. Neither reads Constants — the generator used only the path helpers, which stayed behind in Fallout.Build.Shared as FalloutPaths, and MSBuildTasks does not reference Build.Shared at all. The real forcing function is Fallout.Build.Shared itself: it multi-targets netstandard2.0, its own code (FalloutPaths, Notifications) reads these values, and it keeps that target only so the Roslyn generator can reference it. Say so, and point the exit condition at Fallout-build#442/Fallout-build#441 where that actually gets resolved. netstandard2.1 goes: Core has no conditional compilation and no 2.1-only API, so that asset was IL-identical to the netstandard2.0 one and a 2.1 consumer resolves netstandard2.0 anyway. Verified by dropping it — solution builds clean and the full suite passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…a spec Constants shipped in Fallout.Core.dll but declared `namespace Fallout.Common` — the innermost ring owning a type in an outer layer's namespace. The reference direction was legal, so neither dependency fitness test could see it. Root `Fallout` rather than `Fallout.Core`: every call site already sits somewhere under `Fallout`, so unqualified `Constants.X` resolves by the normal enclosing- namespace walk with no using at all. Only the `using static` form had to change. Adds Core_declares_no_type_in_an_outer_layer_namespace, which pins the rule: Core may declare types under Fallout.Core.* or the root Fallout namespace, and nowhere else. It immediately caught a second offender — ExecutionStatus and ITargetModel, lifted into Core by the de-statification work, still sit in Fallout.Common.Execution. Both are public, so correcting them is a breaking change batched to the yearly major (AGENTS.md rule #1); the namespace is frozen in an explicit baseline the rule ratchets against. Also fixes the dangling `<see cref="FalloutPaths"/>` on Constants — FalloutPaths lives in Fallout.Build.Shared, which Core cannot see, so the cref emitted three CS1574 warnings, one per target framework. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion FalloutGlobalToolPackageId was added but never used — UpdateNotificationAttribute still printed the id as a literal, so the constant was dead and free to drift from the thing it claimed to mirror. Now that Fallout-build#581 has landed and <PackageId> is settled as the singular Fallout.GlobalTool, the two agree and the attribute can read the constant. This is still a hand-kept copy, not a derivation: the attribute ships inside Fallout.Common and runs in a consumer's build, where no Fallout csproj exists to read <PackageId> from. Flowing the id in at compile time is the follow-up on Fallout-build#584. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChrisonSimtian
force-pushed
the
core/constants
branch
from
July 28, 2026 22:40
6a23335 to
d61a653
Compare
5 tasks
Collaborator
Author
|
we've been talking about it, now here it is: lets move all those hidden but very important (in the voice of donald trump) constants that define which versions and package ids to use into a more central place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step of #584. Core is the innermost project, so it is where Fallout's own fixed values belong, and everything now reads them from there. Rebased on #581, so
<PackageId>is settled.The split
Constants.cswas two things in one file. Split on the line between data and behaviour:Fallout.Core/Constants.csconstvalues — directory and file names, parameter names, env var keys, package ids, project URLsFallout.Build.Shared/FalloutPaths.csFallout.Build.Sharednow references Core, so the values have one home and the helpers consume them from it.Namespace
Constantssits in the rootFalloutnamespace, notFallout.Commonand notFallout.Core. Core is the innermost ring and must not declare types under an outer layer's namespace — that inverts the onion in naming even when the reference direction is legal, which is exactly the case the existing dependency fitness tests cannot see.Root
Falloutcosts nothing at the call sites: they all sit somewhere underFallout, so unqualifiedConstants.Xresolves by the normal enclosing-namespace walk. Only theusing staticform changed.New spec
Core_declares_no_type_in_an_outer_layer_namespacepins it. It caught a second, pre-existing offender on the first run:ExecutionStatusandITargetModel, lifted into Core by the de-statification work, still live inFallout.Common.Execution. Both are public, so correcting them is a breaking change and is batched to the yearly major per AGENTS rule #1.Fallout.Common.Executionis frozen in an explicit baseline the rule ratchets against — a list to shrink, never to grow.netstandard2.0 on Core — the real reason
The first pass justified this with "Fallout.SourceGenerators and Fallout.MSBuildTasks consume these values". Both are wrong, and the comment on the csproj now says so correctly:
GetDefaultParametersFile,TryGetRootDirectoryFrom), and those stayed behind inFallout.Build.Shared.Fallout.MSBuildTasksnever referencedFallout.Build.Sharedand reads no constant.The actual forcing function is
Fallout.Build.Shareditself: it multi-targetsnetstandard2.0, its own code (FalloutPaths,Notifications) reads these values, and it keeps that target only so the Roslyn generator can reference it. That matters because it is what makes the exit condition real — when #442 frees the SourceGenerators cone, Build.Shared dropsnetstandard2.0and this target goes with it (#441).netstandard2.1is dropped, not added. Core has no conditional compilation and no 2.1-only API, so that asset was IL-identical to the netstandard2.0 one, and a 2.1 consumer resolves netstandard2.0 anyway. Core now targetsnetstandard2.0;net10.0— one fewer target than before this PR.What could not move
The path helpers (
TryGetRootDirectoryFrom,GetFalloutDirectory,GetDefaultParametersFile,GetCredentialStoreName, …) needAbsolutePath,EnvironmentInfoand the file system. Core references nothing, so they stay inFallout.Build.Shared. No constant was left behind.Tool package id
Adds
FalloutGlobalToolPackageIdand uses it —UpdateNotificationAttributepreviously printed the id as a literal. Still a hand-kept copy rather than a derivation: the attribute ships insideFallout.Commonand runs in a consumer's build, where no Fallout csproj exists to read<PackageId>from. Flowing it in at compile time is the follow-up on #584.Still open from #584
Constants.csonly.docs/agents/conventions.mddoes not yet say where a new setting goes.Verification
dotnet build fallout.slnxclean, 0 errors; Core emits net10.0 + netstandard2.0.CS1574warnings from the first pass are gone — the<see cref="FalloutPaths"/>onConstantspointed at a type Core cannot see and fired once per target framework.🤖 Generated with Claude Code