Skip to content
This repository was archived by the owner on Oct 22, 2025. It is now read-only.

chore: make default runner name 'default'#1369

Closed
MasterPtato wants to merge 1 commit into10-08-fix_fix_serverless_config_req_handle_serverless_sse_closefrom
10-08-chore_make_default_runner_name_default_
Closed

chore: make default runner name 'default'#1369
MasterPtato wants to merge 1 commit into10-08-fix_fix_serverless_config_req_handle_serverless_sse_closefrom
10-08-chore_make_default_runner_name_default_

Conversation

@MasterPtato
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 8, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivetkit-serverless Ready Ready Preview Comment Oct 8, 2025 11:26pm

@claude
Copy link

claude bot commented Oct 8, 2025

Claude finished @MasterPtato's task —— View job


Code Review for PR #1369

Reviewing this pull request that changes the default runner name from "rivetkit" to "default".

Todo List:

  • Read and analyze changed files
  • Check code quality and best practices
  • Look for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage
  • Provide comprehensive feedback

Summary

This PR makes a simple but consistent change across 4 files to standardize the default runner name from "rivetkit" to "default". This is a cosmetic/naming improvement with minimal risk.

Detailed Analysis

Code Quality & Best Practices: ✅ Good

  • The changes are consistent across all locations
  • The createApiActor function signature improvement (adding runnerName parameter with default) is good design
  • Code formatting fixes in router.ts improve readability
  • All changes follow existing patterns and conventions

Potential Bugs or Issues: ✅ None identified

  • No functional logic changes that could introduce bugs
  • All hardcoded "rivetkit" references properly updated to use the configurable parameter
  • The default parameter value ensures backward compatibility

Performance Considerations: ✅ Neutral

  • No performance impact - this is purely a string value change
  • The function signature changes don't add any computational overhead

Security Concerns: ✅ None

  • No security implications from changing runner name defaults
  • No sensitive data exposure or authentication changes

Test Coverage: ✅ Adequate

  • Test files properly updated to reflect the new default
  • The change is covered by existing test suites (driver tests)

Specific Code Changes Reviewed

  1. packages/rivetkit/src/client/config.ts:32

    • ✅ Proper fallback chain: RIVET_RUNNER env var → "default"
    • Good that environment variable still takes precedence
  2. packages/rivetkit/src/manager/router.ts:658

    • ✅ Excellent refactoring of createApiActor function
    • Now properly accepts runnerName parameter instead of hardcoding
    • Default parameter maintains backward compatibility
    • ✅ All call sites updated to pass runConfig.runnerName
  3. Test files updated consistently

    • ✅ Both driver test files updated to use "default"
    • Maintains test accuracy

Recommendations

  • Approve: This is a clean, low-risk change that improves consistency
  • The refactoring of createApiActor is actually an improvement to the codebase
  • No additional changes needed

Minor Notes

  • The code formatting fixes in router.ts (indentation cleanup) are a nice bonus
  • The change aligns with the pattern of using "default" for namespace as well

This PR is ready to merge. It's a straightforward naming improvement with good implementation.


Copy link
Contributor Author

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 either label to this PR to merge it via the merge queue:

  • merge queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 8, 2025

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/cloudflare-workers@1369

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/framework-base@1369

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/next-js@1369

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/sql-loader@1369

commit: 6947895

@graphite-app
Copy link

graphite-app bot commented Oct 9, 2025

Merge activity

  • Oct 9, 12:47 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Oct 9, 12:48 AM UTC: CI is running for this pull request on a draft pull request (#1370) due to your merge queue CI optimization settings.
  • Oct 9, 12:49 AM UTC: Merged by the Graphite merge queue via draft PR: #1370.

graphite-app bot pushed a commit that referenced this pull request Oct 9, 2025
@graphite-app graphite-app bot closed this Oct 9, 2025
@graphite-app graphite-app bot deleted the 10-08-chore_make_default_runner_name_default_ branch October 9, 2025 00:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant