Skip to content

Conversation

@crazyuploader
Copy link
Contributor

No description provided.

@crazyuploader crazyuploader self-assigned this Nov 4, 2025
@crazyuploader crazyuploader requested a review from a team as a code owner November 4, 2025 14:49
@vercel
Copy link

vercel bot commented Nov 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
signoz-web Ready Ready Preview Comment Nov 4, 2025 3:14pm

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

🤖 Hi @crazyuploader, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@claude
Copy link
Contributor

claude bot commented Nov 4, 2025

Claude finished @crazyuploader's task —— View job


Review Complete

Todo List:

  • Read CONTRIBUTING.md for review guidelines
  • Read the changed file
  • Review the changes using git diff
  • Provide review feedback

Summary

This PR enhances the metadata validation script to enforce a 7-day window for document dates (both past and future). The changes are well-structured and improve the validation logic.

Code Review

scripts/check-docs-metadata.js:176-192

The implementation is solid with good improvements:

Good additions:

  • Clear comments explaining date boundaries
  • Symmetric validation for both past and future dates
  • Consistent error messaging format
  • Proper use of else if to avoid duplicate error messages

Code quality:

  • The logic correctly normalizes today by setting time to midnight (line 178)
  • Date calculations are straightforward and maintainable
  • Error messages are clear and actionable

Minor observation (not blocking):
The date validation now enforces a strict 7-day window. This means:

  • Dates older than 7 days will fail validation
  • This could impact backfilling historical documentation or updating old docs

This appears intentional based on the CONTRIBUTING.md requirements (line 43: "Metadata: run yarn check:docs-metadata locally to see missing/invalid entries"), but worth confirming this aligns with the team's workflow expectations.

PR Checklist Reference

Since this is a script/tooling change (not documentation), most items from the PR Checklist in CONTRIBUTING.md don't apply. The relevant checks are:

  • ✅ Change is focused and has a clear purpose
  • ✅ Code follows existing patterns in the file
  • ✅ Error messages are clear and actionable

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

📋 Docs Review Summary

  • Scope: documentation-only review. Non-docs changes were ignored.
  • The changes to the documentation metadata validation script were reviewed. No issues were found based on the current guidelines in CONTRIBUTING.md.

✅ PR Checklist Reference (from CONTRIBUTING.md)

  • No documentation content was changed, so the checklist is not applicable.

🔍 General Feedback

  • No general feedback.

Signed-off-by: Jugal Kishore <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

🤖 Hi @crazyuploader, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@claude
Copy link
Contributor

claude bot commented Nov 4, 2025

Claude finished @crazyuploader's task —— View job


Todo List

  • Read CONTRIBUTING.md for review guidelines
  • Read the changed files
  • Review the changes using git diff
  • Provide review feedback

Summary

This PR improves the documentation metadata validation script by adding a 7-day window constraint for document dates (both past and future). The implementation is solid with comprehensive test coverage. No issues found.

Review

scripts/check-docs-metadata.js:176-195

The date validation logic is well-implemented:

Correct implementation:

  • Properly normalizes dates by setting time to midnight to avoid timezone issues (lines 177-181)
  • Creates symmetric 7-day windows for both past and future dates (lines 184-189)
  • Uses clear variable names (maxFutureDate, minPastDate)
  • Provides informative error messages
  • Uses else if to avoid duplicate error messages (line 193)

Edge cases handled:

  • The boundary logic correctly uses > and < comparisons, making the 7-day boundaries inclusive (lines 191, 193)
  • Test coverage confirms edge cases: exactly 7 days past/future are allowed, 8+ days trigger errors

tests/docs-metadata.test.js

Excellent test coverage:

  • Tests updated to use dynamic dates instead of hardcoded 2024-01-15, preventing future test failures
  • New tests cover all edge cases:
    • Dates exactly at 7-day boundaries (lines 279-319)
    • Dates beyond 7-day boundaries (lines 218-236)
    • Today's date (lines 259-277)
    • Dates within the 7-day window (lines 238-256, 197-216)

Notes

This validation enforces a strict 7-day window for document dates. This means:

  • Documents with dates older than 7 days will fail validation
  • This could impact workflows like backfilling historical docs or batch updates of old documentation

Based on CONTRIBUTING.md line 43, this aligns with the intended workflow where contributors run yarn check:docs-metadata locally before committing.


@makeavish makeavish merged commit 0b6d46a into main Nov 5, 2025
19 checks passed
@makeavish makeavish deleted the fix/github-docs-metadata-workflow branch November 5, 2025 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants