Skip to content

Conversation

NathanFlurry
Copy link
Member

Changes

Copy link
Member Author

NathanFlurry commented Apr 25, 2025

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.

Copy link
Contributor

graphite-app bot commented Apr 25, 2025

Merge activity

  • Apr 25, 12:14 PM EDT: NathanFlurry added this pull request to the Graphite merge queue.
  • Apr 25, 12:15 PM EDT: CI is running for this pull request on a draft pull request (#2395) due to your merge queue CI optimization settings.
  • Apr 25, 12:16 PM EDT: Merged by the Graphite merge queue via draft PR: #2395.

graphite-app bot pushed a commit that referenced this pull request Apr 25, 2025
<!-- Please make sure there is an issue that this PR is correlated to. -->

## Changes

<!-- If there are frontend changes, please include screenshots. -->
@graphite-app graphite-app bot closed this Apr 25, 2025
@graphite-app graphite-app bot deleted the 04-25-docs_clean_up_separation_between_actors_containers_functions branch April 25, 2025 16:16
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR reorganizes the documentation to establish clearer distinctions between Rivet's three main components: Actors, Containers, and Functions.

  • Added new /docs/functions.mdx introducing serverless function capabilities with configuration and routing details
  • Updated site/_redirects to map legacy runtime URLs to their new counterparts (/docs/javascript-runtime/docs/actors, /docs/container-runtime/docs/containers)
  • Added route-related API documentation in /site/src/content/docs/api/routes/ for list, update, and delete operations
  • Modified generateApi.js to include routes-related API endpoints in documentation generation
  • Renamed and restructured documentation files to use consistent terminology across actors, containers, and functions

15 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile

/>
<ExampleLink
href="docs/functions"
title="Rivet Functions"
size="md"
icon={faTs}
Copy link

Choose a reason for hiding this comment

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

logic: using faTs icon for Functions section seems incorrect - should use faFunction icon instead

Comment on lines 17 to 34
<ExampleLink
href="docs/quickstart/typescript"
title="TypeScript"
href="docs/actors"
title="Rivet Actors"
size="md"
icon={faActors}
/>
<ExampleLink
href="docs/containers"
title="Rivet Containers"
size="md"
icon={faServer}
/>
<ExampleLink
href="docs/functions"
title="Rivet Functions"
size="md"
icon={faTs}
/>
Copy link

Choose a reason for hiding this comment

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

style: grid layout with 3 items in 2 columns will result in uneven distribution - consider adjusting grid-cols-2 to grid-cols-3 or grid-cols-1

Comment on lines +28 to +30
await RIVET.routes.delete({
// Add your request body here
});
Copy link

Choose a reason for hiding this comment

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

logic: Example code is missing required 'id' parameter which is marked as required in schema

Suggested change
await RIVET.routes.delete({
// Add your request body here
});
await RIVET.routes.delete({
id: "route-id", // Required: The ID of the route to delete
});


// Make request
await RIVET.routes.list({
// Add your request body here
Copy link

Choose a reason for hiding this comment

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

logic: Example request body should be removed since this GET endpoint doesn't accept a request body

Suggested change
// Add your request body here
// Optional query parameters: project, environment


export default {
async start(ctx: ActorContext) {
// Get the port from environment variables or use a default
const port = parseInt(process.env.PORT_HTTP || "8080");
const port = parseInt(Deno.env.get("PORT_HTTP") || "8080");
Copy link

Choose a reason for hiding this comment

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

logic: Using Deno.env.get() but npm installing @rivet-gg/actor - need to clarify if this is a Deno or Node.js environment

<CodeGroup title='Request' tag='GET' label='https://api.rivet.gg/routes'>

```bash {{ "title": "cURL" }}
curl -X GET 'https://api.rivet.gg/routes'
Copy link

Choose a reason for hiding this comment

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

style: Missing project and environment query parameters in cURL example

| `path` | The URL path to route requests to (cannot end with `/`) |
| `route_subpaths` | When true, routes requests to this path and all nested subpaths to this function |
| `strip_prefix` | When true, removes the configured path prefix from the request URL before forwarding to your function |
| `resources.cpu` | CPU allocation for the function (defaults to 1) |
Copy link

Choose a reason for hiding this comment

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

logic: CPU value of 1 is ambiguous - should clarify if this means 1 core (1000 millicores) or 1 millicore

const { Hono } = require('hono');
const { serve } = require('hono/node-server');
const app = new Hono();
const port = process.env.PORT_HTTP || 8080;
Copy link

Choose a reason for hiding this comment

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

style: PORT_HTTP environment variable usage should be consistent with networking.internal_port config option mentioned later

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.

1 participant