-
Notifications
You must be signed in to change notification settings - Fork 432
Add changelog script and diff url #1725
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.
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
Conversation
Fetch commit details for each desktop_v1 tag to capture the commit author date as createdAt. The change filters tags to desktop_v1, queries the commits API for each tag's SHA, and attaches commit.commit.author.date to the returned GithubTagInfo. This enables consumers to know when a tag was generated. The implementation also adds types for the commit response and logs the fetched tags for debugging. Add changelog metadata utilities and UI diff link Provide structured metadata for changelogs (beforeVersion, newerSlug, olderSlug) by adding a changelog utility that parses versions, handles pre-releases, and builds navigation + diff information. This replaces ad-hoc sorting in route loaders with getChangelogList/getChangelogBySlug and wires a GitHub diff link into the changelog view so users can quickly compare releases. update contents vv Write versioning data into changelog frontmatter Update versioning script to persist fetched version list into the project's changelog (create file if missing). This change adds fs helpers and path join imports and ensures the script updates or inserts the `created` field in the changelog frontmatter rather than overwriting the entire file, so release metadata is preserved and available for downstream processes. v v v
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe PR centralizes changelog metadata computation via a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Route as Changelog Route
participant Module as changelog.ts
participant Content as Changelog Files
User->>Route: Request changelog slug
Route->>Module: getChangelogBySlug(slug)
Module->>Content: Load all changelogs (cached)
activate Module
Note over Module: Parse versions with semver<br/>Build navigation chains<br/>(beforeVersion, newerSlug, olderSlug)
Module-->>Module: Cache results
deactivate Module
Module-->>Route: ChangelogWithMeta + diffUrl
Route-->>User: Render changelog with navigation & diff link
sequenceDiagram
participant Script as versioning.ts
participant GitHub as GitHub API
participant Disk as File System
Script->>GitHub: Fetch all desktop tags
activate GitHub
Note over GitHub: Return tag list with SHA
deactivate GitHub
loop For each tag
Script->>GitHub: Fetch commit date for SHA
activate GitHub
Note over GitHub: Return commit author date
deactivate GitHub
Script->>Script: Enrich GithubTagInfo with createdAt
end
Script->>Script: Build changelog content for each version
Script->>Disk: Create/Update .mdx files<br/>with created timestamps
activate Disk
Note over Disk: Write to content/changelog/
deactivate Disk
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/web/content/changelog/1.0.0-nightly.3.mdx (1)
1-5: Changelog entry and timestamp look good; check possible typoFrontmatter and entry structure look correct. Consider double-checking the wording in Line 5 — if this refers to the Tiptap editor,
titapis likely a typo.apps/web/src/routes/_view/changelog/$slug.tsx (1)
6-29: Slug-based lookup and diff URL wiring look solidThe loader’s switch to
getChangelogBySlug,newerSlug/olderSlugnavigation, and the computed GitHub compare URL (beforeVersion→version) is cohesive and keeps route logic simple. The conditional “View diff on GitHub” link correctly guards ondiffUrland targets a new tab. If you want to harden the external link further, you could changerel="noreferrer"torel="noreferrer noopener"for extra safety, but it’s optional.Also applies to: 34-35, 78-89
apps/web/src/scripts/versioning.ts (2)
30-37: GitHub tag + commit fetch pipeline is correct but consider rate limits and options usageThe enriched
GithubTagInfowithcreatedAtand the commit-fetch loop look correct and match the GitHub API shapes. A couple of non-blocking points to consider:
- You make 1 (tags) + N (commits) GitHub requests in parallel via
Promise.all. With many tags and no token, this can quickly hit unauthenticated rate limits.- The
options?.tokenparameter is wired into headers but the only call site at Line 145 does not pass a token, so the token support may never be exercised.If this script is intended for CI or repeated local use, you may want to pass a GitHub token from the environment and/or limit concurrency (e.g., with a small pool) to make it more robust against API throttling.
Also applies to: 46-108
110-125: Frontmatter update logic works; refine behavior for missing files and error handling
updateCreatedFieldcorrectly detects/creates frontmatter and rewrites thecreatedline while preserving the rest of the header and body.updateChangelogFilesthen:
- Resolves
apps/web/content/changelogfrom the script directory (relative path looks correct for this file).- Reads each
<version>.mdx, defaulting to""if it doesn’t exist, and writes updated content.Two recommended tweaks:
Avoid silently creating empty MDX files
WhenreadFilefails, the code currently generates a new file containing only frontmatter. If you only want to annotate existing changelogs, you could skip writes whenreadFilefails, or at least log that the changelog file is missing instead of creating an empty stub.Propagate failures from the top-level promise
fetchGithubDesktopTags().then(updateChangelogFiles);does not attach a.catch, so network or I/O errors may surface as an unhandled rejection rather than a clear non‑zero exit. Wrapping this in an async main with try/catch (or adding.catch(err => { console.error(err); process.exit(1); })) would make failures explicit for CI or scripting.These are behavior/operational improvements; the current implementation is functionally correct for existing files.
Also applies to: 127-145
apps/web/src/changelog.ts (3)
20-27: Consider simplifying the filter predicate.The type predicate is correct but verbose. You can simplify it slightly:
- .filter( - ( - entry, - ): entry is { - doc: Changelog; - version: semver.SemVer; - } => entry !== null, - ); + .filter((entry): entry is { doc: Changelog; version: semver.SemVer } => + entry !== null + );
63-66: Consider using if/else instead of early return in forEach.Early returns in
forEachwork but reduce readability. An if/else structure would be clearer:indices.forEach((idx, j) => { - if (j === 0) { - withMeta[idx].beforeVersion = null; - return; - } - - const prevIdx = indices[j - 1]; - withMeta[idx].beforeVersion = parsed[prevIdx].doc.version; + if (j === 0) { + withMeta[idx].beforeVersion = null; + } else { + const prevIdx = indices[j - 1]; + withMeta[idx].beforeVersion = parsed[prevIdx].doc.version; + } });
107-116: Navigation logic is correct.The position-based approach for computing newer/older slugs works well. Consider extracting the index lookups for slightly better readability:
descOrder.forEach((idxInParsed, position) => { - const newerIdx = position > 0 ? descOrder[position - 1] : undefined; - const olderIdx = - position < descOrder.length - 1 ? descOrder[position + 1] : undefined; + const hasNewer = position > 0; + const hasOlder = position < descOrder.length - 1; + const newerIdx = hasNewer ? descOrder[position - 1] : undefined; + const olderIdx = hasOlder ? descOrder[position + 1] : undefined; withMeta[idxInParsed].newerSlug = newerIdx !== undefined ? parsed[newerIdx].doc.slug : null;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/content/changelog/1.0.0-nightly.1.mdx(1 hunks)apps/web/content/changelog/1.0.0-nightly.2.mdx(1 hunks)apps/web/content/changelog/1.0.0-nightly.3.mdx(1 hunks)apps/web/src/changelog.ts(1 hunks)apps/web/src/routes/_view/changelog/$slug.tsx(2 hunks)apps/web/src/routes/_view/changelog/index.tsx(2 hunks)apps/web/src/scripts/versioning.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/routes/_view/changelog/index.tsx (2)
apps/web/src/routes/_view/changelog/$slug.tsx (1)
Route(8-31)apps/web/src/changelog.ts (2)
getChangelogList(132-134)ChangelogWithMeta(4-8)
apps/web/src/routes/_view/changelog/$slug.tsx (1)
apps/web/src/changelog.ts (1)
getChangelogBySlug(136-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (7)
apps/web/content/changelog/1.0.0-nightly.1.mdx (1)
2-2: ISO timestamp frontmatter looks goodThe
createdfield now uses a full ISO 8601 UTC timestamp, which is consistent with the new changelog metadata workflow.apps/web/content/changelog/1.0.0-nightly.2.mdx (1)
2-2: Consistent ISOcreatedtimestampThis
createdvalue matches the new ISO 8601 format and keeps nightly entries consistent.apps/web/src/routes/_view/changelog/index.tsx (1)
4-11: Clean switch to centralized changelog metadataUsing
getChangelogList()in the loader and typingchangelogasChangelogWithMetakeeps this route aligned with the new shared changelog meta utilities and removes local sorting/derivation logic. No issues from this side.Also applies to: 67-68
apps/web/src/changelog.ts (4)
1-8: LGTM!The imports and type definition are clean. The
ChangelogWithMetatype appropriately extendsChangelogwith navigation and versioning metadata.
73-99: Verify the beforeVersion computation logic with comprehensive tests.This logic handles three fallback strategies for stable releases, which is complex:
- Link to previous stable within same major/minor (lines 80-84)
- Link to first pre-release with same base (lines 86-91)
- Link to previous stable globally (lines 93-97)
Please ensure this logic is covered by tests, especially for edge cases like:
- First stable release ever (no previous version)
- First stable in a major/minor with and without pre-releases
- Multiple major/minor versions with varying pre-release patterns
You can add unit tests for
buildChangelogMeta()or integration tests that verify thebeforeVersionfield for various version sequences.
136-140: LGTM! Clean slug lookup.The
.find()approach is simple and correct. For large changelog collections (100+), you could optimize with a Map, but this is likely premature optimization for typical use cases.
122-130: Verify cache invalidation behavior with content-collections in development.The module-level cache is correct for production (static content), but in development with the @content-collections/vite plugin, cache staleness is a legitimate concern. While the plugin likely watches and regenerates content, there's no explicit cache invalidation in the code. If the generated "content-collections" module reloads but changelog.ts doesn't, the cache becomes stale.
Recommended actions:
- Verify if @content-collections/vite triggers module reload on content changes in dev mode
- If not guaranteed, add explicit cache invalidation:
if (import.meta.hot) { import.meta.hot.accept(() => { cache = null; }); }- Or document that server restart is required after content changes in development
No description provided.