fix(ci): this time?#19
Conversation
crazycrank
force-pushed
the
feat/scribe-lithography
branch
from
32fb293 to
e4f9701
Compare
There was a problem hiding this comment.
Pull request overview
This PR appears aimed at fixing CI/local-dev restore behavior by removing the repo’s NuGet.config and instead packaging MSBuild build/ entrypoints into the core BulletsForHumanity.Scribe NuGet package to auto-import Scribe.LocalDev.props/targets for consumers.
Changes:
- Pack
build/<PackageId>.props/targetsplusScribe.LocalDev.*into theBulletsForHumanity.Scribenupkg so NuGet can auto-import LocalDev infra for non-SDK consumers. - Add NuGet auto-import entrypoint files (
BulletsForHumanity.Scribe.props/targets) that chain intoScribe.LocalDev.props/targets. - Remove
NuGet.config(previously used to add a local artifacts package source).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Scribe/Scribe.csproj | Packs LocalDev MSBuild files into the core package’s build/ folder. |
| Scribe/build/BulletsForHumanity.Scribe.props | Adds the early-phase NuGet auto-import entrypoint chaining into Scribe.LocalDev.props. |
| Scribe/build/BulletsForHumanity.Scribe.targets | Adds the late-phase NuGet auto-import entrypoint chaining into Scribe.LocalDev.targets. |
| NuGet.config | Deletes the local artifacts feed configuration. |
| <!-- Pack LocalDev props/targets so NuGet auto-imports them for any consumer | ||
| regardless of whether the project uses Scribe.Sdk or Microsoft.NET.Sdk. | ||
| build/BulletsForHumanity.Scribe.{props,targets} are the PackageId-named | ||
| entry points NuGet auto-imports; they chain into Scribe.LocalDev.*. --> | ||
| <None Include="build\BulletsForHumanity.Scribe.props" Pack="true" PackagePath="build\" /> | ||
| <None Include="build\BulletsForHumanity.Scribe.targets" Pack="true" PackagePath="build\" /> | ||
| <None Include="build\Scribe.LocalDev.props" Pack="true" PackagePath="build\" /> | ||
| <None Include="build\Scribe.LocalDev.targets" Pack="true" PackagePath="build\" /> |
There was a problem hiding this comment.
Deleting NuGet.config removes the repo’s only explicit local package source. Because global.json pins the MSBuild SDK BulletsForHumanity.Scribe.Sdk to a timestamped -dev.* version, dotnet restore will typically be unable to resolve the SDK from nuget.org and restores/builds will fail. Consider either (1) keeping a NuGet.config that points to a repo-local .artifacts/packages feed (and ensuring the directory exists before restore), or (2) changing global.json to reference a published SDK version so no local feed is required.
| $(ScribeRoot) and $(ScribesName) must be set by the consumer's | ||
| Directory.Build.props BEFORE this auto-import fires; otherwise the | ||
| file is a no-op. |
There was a problem hiding this comment.
The comment implies both $(ScribeRoot) and $(ScribesName) must be set by consumers for this import to do anything. In Scribe.LocalDev.props, $(ScribesName) is optional for pure consumers (it’s only needed to enable producer mode); requiring it here is misleading. Reword to indicate $(ScribeRoot) is required, and $(ScribesName) is only needed for producer projects.
| $(ScribeRoot) and $(ScribesName) must be set by the consumer's | |
| Directory.Build.props BEFORE this auto-import fires; otherwise the | |
| file is a no-op. | |
| $(ScribeRoot) must be set by the consumer's Directory.Build.props | |
| BEFORE this auto-import fires; otherwise LocalDev consumer behavior is | |
| effectively a no-op. $(ScribesName) is only required for producer | |
| projects. |
No description provided.