Skip to content

Conversation

@MasterPtato
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Jan 7, 2026

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

Project Deployment Review Updated (UTC)
rivet-cloud Ready Ready Preview, Comment Jan 7, 2026 0:54am
rivetkit-serverless Error Error Jan 7, 2026 0:54am
2 Skipped Deployments
Project Deployment Review Updated (UTC)
rivet-inspector Ignored Ignored Preview Jan 7, 2026 0:54am
rivet-site Ignored Ignored Preview Jan 7, 2026 0:54am

Copy link
Contributor Author

MasterPtato commented Jan 7, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link

claude bot commented Jan 7, 2026

PR Review - pkg-pr-new Workflow Updates

Overview

This PR modifies the pkg-pr-new GitHub workflow to adjust which packages are built and published for PR previews.

Changes Summary

  1. Build filter change: '@rivetkit/*''rivetkit'
  2. Published packages: Removed wildcard pattern and added explicit packages
  3. Removed: --template './examples/*' flag

✅ Positive Observations

  1. More explicit package selection: The new approach explicitly lists packages instead of using wildcards, which provides better control and predictability
  2. Additional packages included: Now includes engine/sdks/typescript/api-full/ and shared/typescript/virtual-websocket/ which are dependencies of rivetkit (visible in rivetkit's package.json)

⚠️ Issues & Concerns

1. Missing newline at end of file

The file now ends without a newline character (line 16), which violates POSIX standards and may cause git warnings.

Recommendation: Add a trailing newline at the end of the file.


2. Build filter scope reduction

Before: pnpm build -F '@rivetkit/*' - builds all packages in the @rivetkit scope
After: pnpm build -F 'rivetkit' - builds only the rivetkit package

Potential issue: Looking at the workspace structure, there are other packages in rivetkit-typescript/packages/ such as:

  • rivetkit-typescript/packages/db
  • rivetkit-typescript/packages/react
  • rivetkit-typescript/packages/next-js
  • rivetkit-typescript/packages/cloudflare-workers

If these packages depend on each other or if rivetkit depends on them, the narrower build filter might miss building necessary dependencies.

Questions to consider:

  • Does the rivetkit package have workspace dependencies on other @rivetkit/* packages?
  • Will pnpm's build process automatically handle transitive dependencies, or do they need to be explicitly built?

3. Removal of template examples flag

The --template './examples/*' flag was removed.

Impact: This means pkg-pr-new will no longer generate preview deployments for the examples. If the examples were being used to test/demonstrate the packages in PR previews, this functionality is now lost.

Recommendation: Clarify whether this is intentional. If examples testing is still needed, consider keeping the template flag or document why it's no longer necessary.


4. PR title indicates "DO NOT MERGE"

The PR title explicitly says "DO NOT MERGE", suggesting this is a work-in-progress or experimental change.

Recommendation: Before merging, ensure:

  • The title is updated to remove "DO NOT MERGE"
  • All intended functionality is working as expected
  • The changes have been tested in a real PR preview scenario

🔍 Testing Recommendations

  1. Verify build process: Ensure pnpm build -F 'rivetkit' successfully builds all necessary dependencies
  2. Test pkg-pr-new output: Create a test PR to verify that all required packages are correctly published and accessible
  3. Check dependency resolution: Verify that workspace dependencies (@rivetkit/virtual-websocket, @rivetkit/engine-runner) are correctly included in the published preview

📝 Additional Notes

  • The change from wildcards to explicit paths is generally good for CI/CD predictability
  • Consider documenting why specific packages are included (especially api-full and virtual-websocket) for future maintainers
  • If this workflow is critical for PR previews, add comments explaining the package selection logic

Summary Rating

Code Quality: ⚠️ Good intent, needs verification
Completeness: ⚠️ Unclear if all dependencies are covered
Risk Level: 🟡 Medium - Could break PR preview functionality if dependencies are missing

Recommendation: Do not merge until:

  1. Build filter scope is verified to include all necessary packages
  2. Template removal is confirmed as intentional
  3. File formatting is fixed (newline at EOF)
  4. PR title is updated

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 7, 2026

Open in StackBlitz

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3763

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3763

@rivetkit/engine-api-full

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-api-full@3763

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3763

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3763

commit: c790ee8

@MasterPtato MasterPtato force-pushed the 01-06-chore_do_not_merge_fix_pkg_pr_rivetkit branch from d0fed0e to 0dd135f Compare January 7, 2026 00:11
@MasterPtato MasterPtato force-pushed the 01-06-chore_do_not_merge_fix_pkg_pr_rivetkit branch from 0dd135f to 4428662 Compare January 7, 2026 00:52
@MasterPtato MasterPtato force-pushed the 01-06-fix_fix_dashboards_add_pb_metrics_fix_listen_n_order branch from 63497dc to ed662df Compare January 7, 2026 20:29
@graphite-app graphite-app bot changed the base branch from 01-06-fix_fix_dashboards_add_pb_metrics_fix_listen_n_order to graphite-base/3763 January 7, 2026 20:58
@graphite-app graphite-app bot force-pushed the 01-06-chore_do_not_merge_fix_pkg_pr_rivetkit branch from 4428662 to c0556fa Compare January 7, 2026 20:59
@graphite-app graphite-app bot force-pushed the graphite-base/3763 branch from ed662df to 3618c74 Compare January 7, 2026 20:59
@graphite-app graphite-app bot changed the base branch from graphite-base/3763 to main January 7, 2026 21:00
@graphite-app graphite-app bot force-pushed the 01-06-chore_do_not_merge_fix_pkg_pr_rivetkit branch from c0556fa to 0a33f55 Compare January 7, 2026 21:00
@jog1t jog1t force-pushed the 01-06-chore_do_not_merge_fix_pkg_pr_rivetkit branch from 0a33f55 to c790ee8 Compare January 7, 2026 22:25
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