Ensure static site builds write .nojekyll#61
Conversation
Code Review SummaryThis PR adds the ✅ Strengths
📝 Code QualityWebSiteBuilder.cs (PowerForge.Web/Services/WebSiteBuilder.cs:102-107)
WebSiteAuditOptimizeBuildTests.cs (PowerForge.Tests/WebSiteAuditOptimizeBuildTests.cs:268-318)
🔍 Observations
💡 Minor Suggestions (Optional)
private static void EnsureNoJekyllFile(string outputRoot)
{
// GitHub Pages requires .nojekyll to skip Jekyll processing
var markerPath = Path.Combine(outputRoot, ".nojekyll");
if (!File.Exists(markerPath))
File.WriteAllText(markerPath, string.Empty);
}
private static void EnsureNoJekyllFile(string outputRoot)
{
var markerPath = Path.Combine(outputRoot, ".nojekyll");
File.WriteAllText(markerPath, string.Empty); // Overwrites if exists
}Since the file is always empty, overwriting is harmless and removes a check. ✅ RecommendationLGTM - Approve and merge This is a focused, well-implemented fix with appropriate test coverage. The change is low-risk and solves a real deployment issue. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92d2a0381f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!File.Exists(markerPath)) | ||
| File.WriteAllText(markerPath, string.Empty); |
There was a problem hiding this comment.
Handle existing
.nojekyll directory before writing marker
EnsureNoJekyllFile only checks File.Exists, but File.Exists(".../.nojekyll") returns false when that path is an existing directory, so File.WriteAllText then throws and aborts the build. Because Build does not clean outDir, this can happen on incremental builds if a previous run (or copied content/assets) left a .nojekyll/ directory at the root, turning this new marker step into a hard failure instead of a no-op.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR ensures that static site builds always include a .nojekyll marker file at the output root to prevent GitHub Pages from treating the static output as Jekyll input. This addresses deployment verification failures that occurred when the marker was missing.
Changes:
- Added
EnsureNoJekyllFilemethod to write.nojekyllmarker at the site root during build - Integrated the marker file creation into the
WebSiteBuilder.Buildprocess - Added regression test to verify the marker is always produced
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| PowerForge.Web/Services/WebSiteBuilder.cs | Adds EnsureNoJekyllFile helper method and calls it during the build process to create the .nojekyll marker file |
| PowerForge.Tests/WebSiteAuditOptimizeBuildTests.cs | Adds test Build_WritesNoJekyllMarkerAtSiteRoot to verify the marker file is created during site builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Validation
Why