Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Nov 24, 2025

Changes

@vercel
Copy link

vercel bot commented Nov 24, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-site Building Building Preview Comment Nov 24, 2025 10:22pm
3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 24, 2025 10:22pm
rivet-inspector Ignored Ignored Nov 24, 2025 10:22pm
rivetkit-serverless Skipped Skipped Nov 24, 2025 10:22pm

@jog1t jog1t self-assigned this Nov 24, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 24, 2025

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@jog1t jog1t merged commit 42e8611 into main Nov 24, 2025
10 of 11 checks passed
@jog1t jog1t deleted the fix/giscus-repo branch November 24, 2025 22:22
@claude
Copy link

claude bot commented Nov 24, 2025

Code Review

Summary

This PR updates the Giscus comments repository reference from rivet-dev/engine to rivet-dev/rivet in the Comments component. The change appears to be a necessary correction to point to the correct repository.

✅ Positive Observations

  • Simple, focused change: The PR has a clear, single purpose
  • Correct fix: Based on the current repository structure (this PR is in rivet-dev/rivet), updating the Giscus repo reference to match is the right approach
  • No breaking changes: The repoId remains unchanged, which should maintain comment continuity if the Giscus configuration was migrated properly

⚠️ Issues & Concerns

Critical: Incomplete Migration

This change only updates 1 of 20+ files that reference rivet-dev/engine. Other files still pointing to the old repository include:

  1. website/src/components/GitHubStars.tsx:19 - Default repo parameter
  2. website/src/app/(v2)/(marketing)/cloud/RivetCloudSection.tsx:45 - Button href
  3. website/src/app/(v2)/(marketing)/cloud/CommunitySection.tsx:23 - Discussions link
  4. website/src/app/(v2)/(marketing)/cloud/PowerfulPrimitivesSection.tsx:143 - Example link
  5. Multiple blog posts (10+ files in website/src/posts/)
  6. Documentation (website/src/content/docs/self-hosting/install.mdx)
  7. Build scripts (website/scripts/generateApi.js, website/scripts/generateReadme.mjs)

Impact: This creates an inconsistent state where some components point to the new repo and others to the old one. This could lead to:

  • Broken links if rivet-dev/engine doesn't exist or is archived
  • Confusion for users seeing mixed repository references
  • Incorrect star counts or discussion links

Recommendations

  1. Expand scope: Update ALL references from rivet-dev/engine to rivet-dev/rivet in a single cohesive PR
  2. Verify links: Test that all updated links work correctly (especially the example link with the specific commit hash)
  3. Update PR description: Add context about why this change is needed (repo rename/migration?)
  4. Consider redirects: If rivet-dev/engine is being deprecated, ensure GitHub redirects are in place

🧪 Testing Suggestions

  • Verify Giscus comments still load correctly on the website
  • Check that the repoId corresponds to the rivet-dev/rivet repository in Giscus settings
  • Test the comments feature in a staging environment before deploying

📝 Minor Notes

  • The PR description is quite sparse - adding context about the repo migration would help reviewers understand the change
  • Consider adding a commit that follows the conventional commit format from CLAUDE.md: fix(website): change giscus repo to rivet-dev/rivet

Verdict

Needs Work: While this specific change is correct, it's incomplete. I recommend expanding this PR to fix all occurrences of rivet-dev/enginerivet-dev/rivet for consistency and to avoid future issues.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 24, 2025

Graphite Automations

"Test" took an action on this PR • (11/24/25)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

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.

2 participants