-
Notifications
You must be signed in to change notification settings - Fork 104
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
cosmetic improvements #1276
base: v0.8
Are you sure you want to change the base?
cosmetic improvements #1276
Conversation
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.
Awesome, just a few question on naming.
@@ -1,4 +1,4 @@ | |||
import { createConfig } from "@ponder/core"; | |||
import { createConfig } from "ponder"; |
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.
@@ -1,27 +1,15 @@ | |||
/// <reference types="ponder/virtual" /> | |||
|
|||
declare module "ponder:register" { |
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.
What's the full list of names you considered here? Want to get this right, because it's going to show up in a LOT of code snippets for a long time :)
Edit: Oh, I figured this was the name that would replace "@/generated" but now I see that's different. Do you think we should rename "@/generated" in this release? I lean yes.
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.
Yes I think we should replace @/generated
, but I haven't done it yet.
This ponder:register
is an entirely internal thing that is used to pass type information back into the packages/core/src/types.d.ts
.
import { initTRPC } from "@trpc/server"; | ||
import { eq } from "ponder"; |
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.
Is there any value to using different entrypoints here?
import { eq } from "ponder/db";
// or
import { eq } from "ponder/drizzle";
// or
import { eq } from "ponder/sql";
Everything from top-level seems fine/simple, just curious if you gave it any thought.
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.
Let's take another look on monday
@@ -121,8 +121,7 @@ export function createTelemetry({ | |||
|
|||
// Attempt to find and read the users package.json file. | |||
const packageJson = getPackageJson(options.rootDir); | |||
const ponderCoreVersion = | |||
packageJson?.dependencies?.["@ponder/core"] ?? "unknown"; | |||
const ponderCoreVersion = packageJson?.dependencies?.ponder ?? "unknown"; |
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.
ponderVersion
?
@ponder/core
->ponder
ponder:schema
ponder-env.d.ts
Remaining
@/generated