Skip to content

fix(docs): skip ms.date updates and republish for metadata-only changes #2154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MSBrett wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(docs): skip ms.date updates and republish for metadata-only changes #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/update-mslearn-dates.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,42 @@ jobs:
run: |
# Get current date in MM/DD/YYYY format
CURRENT_DATE=$(date +'%m/%d/%Y')
BASE_SHA="${{ github.event.pull_request.base.sha }}"
echo "Updating ms.date to: $CURRENT_DATE"
echo "Comparing article body changes against: $BASE_SHA"

strip_frontmatter() {
awk '
NR == 1 && $0 == "---" { in_frontmatter = 1; next }
in_frontmatter && $0 == "---" { in_frontmatter = 0; next }
!in_frontmatter { print }
'
}

has_content_changes() {
local file="$1"

if ! git cat-file -e "$BASE_SHA:$file" 2>/dev/null; then
return 0
fi

! cmp -s <(git show "$BASE_SHA:$file" | strip_frontmatter) <(strip_frontmatter < "$file")
}

# Process each changed markdown file
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
echo "Processing: $file"

if [ ! -f "$file" ]; then
echo " File no longer exists, skipping"
continue
fi

if ! has_content_changes "$file"; then
echo " No article body changes found, skipping"
continue
fi

# Check if file has ms.date in frontmatter and update it
if grep -q "^ms\.date:" "$file"; then
# Use sed to replace the ms.date line
Expand Down
16 changes: 16 additions & 0 deletions src/powershell/Tests/Unit/Action.UpdateMsLearnDates.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ Describe 'update-mslearn-dates GitHub Action' {
$workflowContent | Should -Match "date \+'%m/%d/%Y'"
}

It 'Should compare files against the pull request base SHA' {
$workflowContent | Should -Match 'github\.event\.pull_request\.base\.sha'
$workflowContent | Should -Match 'git cat-file -e "\$BASE_SHA:\$file"'
}

It 'Should ignore frontmatter when checking for article body changes' {
$workflowContent | Should -Match 'strip_frontmatter\(\)'
$workflowContent | Should -Match 'in_frontmatter'
$workflowContent | Should -Match 'No article body changes found, skipping'
}

It 'Should compare article body content before updating ms.date' {
$workflowContent | Should -Match 'cmp -s'
$workflowContent | Should -Match 'has_content_changes "\$file"'
}

It 'Should use sed to replace ms.date line' {
$workflowContent | Should -Match 'sed -i'
$workflowContent | Should -Match 's[/|]\^ms\\\.date:'
Expand Down
62 changes: 62 additions & 0 deletions src/scripts/Publish-Toolkit.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ function Set-RepoContent($repo, [string]$branchPrefix, [string]$sourceDir)
}
./New-Directory $repo.path
Get-ChildItem $sourceDir -Exclude .buildignore | Copy-Item -Destination $repo.path -Recurse
if ($Template -eq "docs")
{
Remove-DocsMetadataOnlyChanges $repo.path
}

# Commit changes
if ($Branch)
Expand Down Expand Up @@ -205,6 +209,64 @@ function Set-RepoContent($repo, [string]$branchPrefix, [string]$sourceDir)
Write-Host ''
}

function Get-DocsArticleBody([string]$content)
{
$lines = $content -split "`r?`n"
$start = 0

if ($lines.Count -gt 0 -and $lines[0] -eq "---")
{
for ($i = 1; $i -lt $lines.Count; $i++)
{
if ($lines[$i] -eq "---")
{
$start = $i + 1
break
}
}
}

$ignoreLines = @(
"<!-- markdownlint-disable-next-line MD025 -->",
"<!-- prettier-ignore-start -->",
"<!-- prettier-ignore-end -->"
)

$body = $lines[$start..($lines.Count - 1)] `
Copy link
Copy Markdown
Collaborator

@flanakin flanakin May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [AI][Claude Opus 4.7] 💡 Suggestion

Edge case: if a file is frontmatter-only (closing --- is the last line), $start equals $lines.Count and $lines[$start..($lines.Count - 1)] becomes $lines[Count..(Count-1)], which PowerShell evaluates as a reversed range and returns the last element instead of an empty slice. The function would then return that line (likely empty), which happens to still compare equal across versions, so the visible blast radius is small — but the logic is wrong.

Guard with a length check, e.g.:

Suggested change
$body = $lines[$start..($lines.Count - 1)] `
if ($start -ge $lines.Count)
{
return ""
}
$body = $lines[$start..($lines.Count - 1)] `

(Existing pipeline continues from | Where-Object ....)

| Where-Object { $ignoreLines -notcontains $_.Trim() } `
| ForEach-Object { $_.TrimEnd() }

return (($body -join "`n") -replace "`n{3,}", "`n`n").Trim()
Comment on lines +229 to +239
Copy link
Copy Markdown
Collaborator

@flanakin flanakin May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [AI][Claude Opus 4.7] ⚠️ Should fix

The workflow's strip_frontmatter (awk) only removes the ---...--- block, but Get-DocsArticleBody here additionally strips markdownlint/prettier ignore comments and normalizes trailing whitespace + blank-line runs. The two definitions disagree on "what counts as body."

A PR that changes only an ignore-comment or whitespace is seen as a real body change by the workflow (so ms.date gets bumped), then reverted to HEAD at publish time (with the OLD ms.date). Net result is correct today, but:

  1. The PR carries a wasted ms.date bump that will be silently undone.
  2. The two normalization implementations will drift over time.

Consider a single source of truth — e.g., a small helper script the workflow sources, or matching the workflow's awk to also drop the ignore-comment lines. (Re-raising @RolandKrummenacher's top-level point at the line level for visibility.)

}
Comment on lines +212 to +240
Copy link
Copy Markdown
Collaborator

@flanakin flanakin May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [AI][Claude Opus 4.7] ⚠️ Should fix

Get-DocsArticleBody is pure and easy to unit test, but there are no Pester tests covering it. The new tests in Action.UpdateMsLearnDates.Tests.ps1 regex-match the workflow YAML (fine for the workflow guard) but don't exercise this function at all.

Minimal coverage worth adding (paraphrasing @RolandKrummenacher's suggestion):

It 'Get-DocsArticleBody strips frontmatter'
It 'Get-DocsArticleBody strips markdownlint/prettier ignore lines'
It 'Get-DocsArticleBody collapses 3+ blank lines to 2'
It 'Get-DocsArticleBody trims trailing whitespace per line'
It 'Get-DocsArticleBody returns "" for empty or frontmatter-only input'

If both the workflow's strip_frontmatter and this function are required to satisfy a shared fixture, the asymmetry above becomes visible the moment one drifts.


function Remove-DocsMetadataOnlyChanges([string]$docsPath)
{
Push-Location
Set-Location $docsPath
$repoRoot = git rev-parse --show-toplevel
Set-Location $repoRoot

Comment on lines +244 to +248
Copy link
Copy Markdown
Collaborator

@flanakin flanakin May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [AI][Claude Opus 4.7] 💡 Suggestion

The function changes the process cwd (Set-Location $repoRoot) so that git restore --source HEAD -- $repoFile later resolves $repoFile against the correct repo. It works, but it's implicit — anyone reading the loop has to remember the cwd was changed 15 lines earlier.

Passing -C $repoRoot to each git call would make the intent explicit and remove the need to change cwd:

git -C $repoRoot cat-file -e "HEAD:$repoFile" 2>$null
...
$previousContent = [string]::Join("`n", (git -C $repoRoot show "HEAD:$repoFile"))
...
git -C $repoRoot restore --source HEAD -- $repoFile

Minor — purely a readability/robustness improvement.

Get-ChildItem $docsPath -Recurse -Filter "*.md" | ForEach-Object {
$repoFile = [System.IO.Path]::GetRelativePath($repoRoot, $_.FullName).Replace("\", "/")
git cat-file -e "HEAD:$repoFile" 2>$null

if ($LASTEXITCODE -ne 0)
{
return
}

$previousContent = [string]::Join("`n", (git show "HEAD:$repoFile"))
$currentContent = Get-Content $_.FullName -Raw
Comment on lines +249 to +259

if ((Get-DocsArticleBody $previousContent) -eq (Get-DocsArticleBody $currentContent))
{
git restore --source HEAD -- $repoFile
}
}

Pop-Location
Comment on lines +245 to +267
}

# Get version for branch name and commit message
$ver = & "$PSScriptRoot/Get-Version.ps1"

Expand Down
Loading