fix(scripts): standardize Timestamp JSON key casing across all lint result files#1314
fix(scripts): standardize Timestamp JSON key casing across all lint result files#1314chaosdinosaur wants to merge 14 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1314 +/- ##
==========================================
- Coverage 87.66% 87.65% -0.02%
==========================================
Files 61 61
Lines 9328 9328
==========================================
- Hits 8177 8176 -1
- Misses 1151 1152 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR makes the right change — renaming the remaining lowercase timestamp JSON keys to Timestamp (PascalCase) to match the convention already enforced by Get-StandardTimestamp in LintingHelpers.psm1. The core logic is correct, all four script hashtables and their corresponding test assertions are consistently updated, and no security or functional concerns were found.
However, there are a few items that must be addressed before this can merge.
Issue Alignment
✅ Fixes #1003 — the PR correctly addresses the stated goal of standardizing Timestamp casing.
📌 Scope observation (non-blocking): Issue #1003's "Files Requiring Changes" table listed two files (Invoke-LinkLanguageCheck.ps1 and Markdown-Link-Check.ps1), but the PR also updates Test-CopyrightHeaders.ps1 and Validate-SkillStructure.ps1. This is the right call — those two files had the same inconsistency and should be fixed here. The PR description acknowledges this, and the additional changes are well-scoped and correct. No action needed; just confirming the intentional expansion aligns with the issue's underlying goal.
PR Template Compliance
lint:md and lint:ps). The following six are missing entirely:
| Command | Expected disposition for this PR |
|---|---|
npm run spell-check |
Run and confirm pass (or mark N/A if no prose changed) |
npm run lint:frontmatter |
Run and confirm pass (or mark N/A) |
npm run validate:skills |
Run and confirm pass (or mark N/A) |
npm run lint:md-links |
Run and confirm pass (or mark N/A) |
npm run plugin:generate |
Run and confirm pass (or mark N/A) |
npm run docs:test |
Run and confirm pass (or mark N/A) |
Even when a command is not expected to surface failures for a given change type, the checklist items should be explicitly checked (or annotated N/A) so reviewers can confirm they were considered. Leaving them absent creates ambiguity.
Action required: Re-run the full suite per the template and update the checklist.
Coding Standards
Two inline comments have been posted on the diff for the following issues:
-
Unintentional whitespace / line-ending changes — Five files (
Invoke-LinkLanguageCheck.ps1,Markdown-Link-Check.ps1,Test-CopyrightHeaders.ps1,Invoke-LinkLanguageCheck.Tests.ps1,Test-CopyrightHeaders.Tests.ps1) show a line-1 diff that is visually identical in content. This is a CRLF/LF mismatch or trailing-whitespace artifact introduced during editing. These changes are out of scope and pollute the diff. -
Trailing-newline removal — Two files (
Invoke-LinkLanguageCheck.ps1andValidate-SkillStructure.Tests.ps1) have their final blank line silently removed. This was not declared in the PR scope and should be reverted unless there is a deliberate repo-wide convention to strip trailing newlines.
Code Quality
✅ No bugs, security issues, or logic errors found. The renames are mechanical, complete, and correct across all four production scripts and all five test files.
Action Items
- Revert the whitespace-only line-1 changes in the five affected files to eliminate CRLF/trailing-space noise.
- Restore the trailing blank lines in
Invoke-LinkLanguageCheck.ps1andValidate-SkillStructure.Tests.ps1. - Complete the Required Automated Checks checklist in the PR description — run all eight commands and check (or mark N/A) each entry.
Addressing Review FeedbackAll three action items from the automated review have been addressed in commit e06aabd: 1. ✅ Whitespace / line-ending changes — FixedRoot cause identified as UTF-8 BOM stripping. All seven modified files had a 2. ✅ Trailing newline removal — FixedTrailing blank lines restored in 3. ℹ️ PR Template Compliance — NotedThe six missing automated check entries ( Result: The combined diff against |
Template Compliance Check ResultsAll template compliance checks pass on branch ✅ PSScriptAnalyzer Lint — PASSAll PowerShell files pass ✅ UTF-8 BOM Compliance — PASS
✅ Timestamp Key Casing — PASSAll hashtable key assignments standardized from ✅ Pester Tests — PASSAll linting tests pass with the updated |
|
@chaosdinosaur ... we could also move this I guess to a CI Helper function ... thoughts? |
|
@WilliamBerryiii Good thought — here's a quick analysis:
If the suggestion is a higher-level helper (e.g.,
Recommendation: Keep as-is for this PR. If a second standard field emerges later (e.g., |
|
Update: @WilliamBerryiii raised a valid follow-up concern — the ISO 8601 timestamp validation regex ( This is better addressed by exporting a |
There was a problem hiding this comment.
Advisory review — this PR is from a maintainer. Findings are informational only.
Review Summary
PR #1314 makes clean, focused, mechanical changes: renaming the timestamp hashtable key to Timestamp (PascalCase) in four linting scripts and their corresponding Pester test files. The implementation is correct and well-targeted. Two advisory observations are noted below.
Issue Alignment ✅
The PR clearly links Fixes #1003 and the changes directly address the issue’s stated goal of standardizing Timestamp casing across lint result JSON files. The scope is reasonable, and the intentional exclusion of three scripts using inline [datetime]::UtcNow is documented in Additional Notes.
PR Template Compliance ⚠️
Five required automated checks are unchecked in the checklist:
npm run spell-checknpm run lint:frontmatternpm run validate:skillsnpm run lint:md-linksnpm run plugin:generate
The template labels these as required before merging. For a pure PowerShell script change, most are unlikely to surface regressions, but they should be run and checked off (or explicitly marked N/A) to satisfy the template requirement.
Coding Standards ✅
All changed files comply with the PowerShell conventions in .github/instructions/. Timestamp uses correct PascalCase, copyright headers are untouched, and the test assertions are updated consistently.
Code Quality — One Scope Observation 💡
scripts/tests/linting/FrontmatterValidation.Tests.ps1 lines 355–356 were not updated.
FrontmatterValidation.psm1 already uses Timestamp (PascalCase) in its ToHashtable() method (line 197 on main). The corresponding test still asserts the lowercase key:
$hash.ContainsKey('timestamp') | Should -BeTrue
$hash['timestamp'] | Should -Match '^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z$'The tests pass today because PowerShell @{} hashtables are case-insensitive, so .ContainsKey('timestamp') finds Timestamp. However, since the PR’s stated goal is to standardize casing across all lint result files, updating these assertions would complete the picture and accurately reflect the key name in serialized JSON output.
Suggested fix for scripts/tests/linting/FrontmatterValidation.Tests.ps1 lines 355–356:
# Before
$hash.ContainsKey('timestamp') | Should -BeTrue
$hash['timestamp'] | Should -Match '^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z$'
# After
$hash.ContainsKey('Timestamp') | Should -BeTrue
$hash['Timestamp'] | Should -Match '^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z$'This file is not in the current diff; the fix can be included in this PR or as a follow-up commit.
Action Items
- Run the five unchecked automated checks and tick them off (or mark as N/A).
- Consider updating
FrontmatterValidation.Tests.ps1lines 355–356 to use'Timestamp'for consistency with the PR’s standardization goal.
There was a problem hiding this comment.
Advisory review, this PR is from a maintainer. Findings are informational only.
Review Summary
This PR makes a clean, well-scoped fix — renaming the timestamp hashtable key to Timestamp (PascalCase) in four linting scripts and their corresponding Pester test files. The implementation is correct, all eight files are consistently updated, and the tests accurately reflect the new casing. The intent aligns with the stated goal of completing the final batch of standardization work for issue #1003.
Two minor informational notes follow.
✅ Issue Alignment
The changes directly and completely address the stated goal of standardizing timestamp → Timestamp across the four remaining linting scripts and their test files, completing the series started in issues #994–#1002.
⚠️ PR Template Compliance
The PR description omits several entries from the Required Automated Checks section of the template:
| Check | Status in PR |
|---|---|
npm run lint:md |
✅ checked |
npm run lint:ps |
✅ checked |
npm run spell-check |
❌ unchecked |
npm run lint:frontmatter |
❌ unchecked |
npm run validate:skills |
❌ unchecked |
npm run lint:md-links |
❌ unchecked |
npm run plugin:generate |
❌ unchecked |
npm run docs:test |
❌ not listed (present in template) |
Several of these (frontmatter, skills, links, plugin, docs) are genuinely not applicable to pure .ps1 changes. However, the template lists all of them as "Required before merging," and leaving them unchecked without explanation makes it unclear whether they were run and passed or simply skipped. For future PRs in this series, consider checking non-applicable items with an (N/A — no .md/.yml/plugin changes) annotation, or confirming a full npm run lint:all pass, to keep the checklist unambiguous for reviewers.
💡 BOM Side Effect (Informational)
The initial fix commits accidentally stripped UTF-8 BOM bytes and trailing newlines from seven files; this was caught and corrected in commit e06aabd (fix(scripts): restore UTF-8 BOM and trailing newlines stripped during edits). The final file state is correct. No action required — just a note that configuring the editor (e.g., VS Code files.encoding: utf8bom, .editorconfig charset = utf-8-bom) to preserve BOM on save would prevent this class of noise commit in future edits to these files.
💡 Remaining Inconsistencies (Out of Scope — Noted)
As documented in the PR's Additional Notes, two security scripts still use ad-hoc datetime formatting instead of Get-StandardTimestamp:
scripts/security/Test-SHAStaleness.ps1(line 493):Get-Date -Format "yyyy-MM-ddTHH:mm:ssZ"scripts/security/Test-ActionVersionConsistency.ps1(line 252):(Get-Date).ToString('yyyy-MM-ddTHH:mm:ss.fffZ')
The exclusion is intentional and clearly explained. Tracking follow-up issues for these two scripts would be a good next step to complete the full standardization.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
chaosdinosaur
dismissed
github-actions[bot]’s stale review
need to reset github-actions bot review
4 participants
Pull Request
Description
Standardizes the
timestampJSON key casing toTimestamp(capital T) across the four remaining linting scripts and their corresponding Pester test files. Issues #994–#1002 previously standardized the other scripts; this PR completes the final batch for issue #1003.The change ensures every lint result JSON file uses
Timestamp = Get-StandardTimestampwith consistent PascalCase key naming, matching the convention established byGet-StandardTimestampinLintingHelpers.psm1.Fixes #1003
Related Issue(s)
Fixes #1003
Type of Change
Select all that apply:
Code & Documentation:
Other:
.ps1,.sh,.py)Testing
Timestamp(capital T) instead oftimestamptimestampkeys in the modified scripts via grepFiles Changed
Scripts (hashtable key
timestamp→Timestamp):scripts/linting/Invoke-LinkLanguageCheck.ps1— lines 87, 141scripts/linting/Markdown-Link-Check.ps1— line 322scripts/linting/Test-CopyrightHeaders.ps1— line 242scripts/linting/Validate-SkillStructure.ps1— line 494Tests (assertions updated to match new casing):
scripts/tests/linting/Invoke-LinkLanguageCheck.Tests.ps1— lines 133, 187scripts/tests/linting/Markdown-Link-Check.Tests.ps1— line 352scripts/tests/linting/Test-CopyrightHeaders.Tests.ps1— lines 594, 603scripts/tests/linting/Validate-SkillStructure.Tests.ps1— line 827Checklist
Required Checks
Required Automated Checks
npm run lint:mdnpm run lint:psnpm run spell-check— N/A, no prose changes; verified passnpm run lint:frontmatter— N/A, no markdown frontmatter changes; verified passnpm run validate:skills— N/A, no skill changes; verified pass (17 skills, 0 errors)npm run lint:md-links— N/A, no markdown changes; verified passnpm run plugin:generate— N/A, no plugin-affecting changesnpm run docs:test— N/A, no documentation changesSecurity Considerations
Additional Notes
Three additional scripts (
Test-ActionVersionConsistency.ps1,Test-SHAStaleness.ps1,Invoke-PesterTests.ps1) use inline[datetime]::UtcNowinstead ofGet-StandardTimestampand were intentionally left out of this PR per user direction. They may warrant separate issues to adoptGet-StandardTimestamp."