fix: use by over for for Resolver.records_ arg#1934
Conversation
🦋 Changeset detectedLatest commit: 8fcaf67 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 36 minutes and 35 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Standardizes the Omnigraph GraphQL API argument naming by renaming Resolver.records_’s lookup argument from for to by, and updates the generated schema artifacts accordingly.
Changes:
- Rename
Resolver.records_(for:)→Resolver.records_(by:)in the Omnigraph API resolver implementation. - Regenerate enssdk’s published GraphQL schema (
schema.graphql) and gql.tada introspection to reflect the new argument name. - Add a changeset noting the (intentional) breaking API change for
ensapiandenssdk.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/enssdk/src/omnigraph/generated/schema.graphql | Updates the public schema definition to use records_(by: ...). |
| packages/enssdk/src/omnigraph/generated/introspection.ts | Updates gql.tada introspection so generated client typing matches the schema change. |
| apps/ensapi/src/omnigraph-api/schema/resolver.ts | Implements the argument rename in the server schema/resolver definition. |
| .changeset/fix-records-by-arg.md | Records the breaking API rename for release notes/versioning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryRenames the Confidence Score: 5/5Safe to merge — the rename is clean and all active generated artifacts are consistent. All remaining findings are P2: a SemVer bump classification question in the changeset, and a stale sibling generated file that nothing imports. Neither blocks correctness or type-safety of the active code paths.
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant GraphQL as Omnigraph API
participant Resolver as resolver.ts
Client->>GraphQL: query { resolver { records_(by: { name: "foo.eth" }) } }
Note over GraphQL: arg renamed from `for` to `by`
GraphQL->>Resolver: resolve({ chainId, address }, args)
Resolver->>Resolver: args.by.node ?? namehashInterpretedName(args.by.name)
Resolver-->>Client: ResolverRecords
Reviews (1): Last reviewed commit: "fix: use `by` over `for` for Resolver.re..." | Re-trigger Greptile |
Standardizes the Omnigraph API on `by` for id lookups. Closes #1904.
0262be9 to
e8ee9ad
Compare
No longer needed; gql.tada now writes only introspection.ts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Resolver.records_(for:)toResolver.records_(by:)in the Omnigraph API so every id lookup uses the samebyargument convention.schema.graphql+ gql.tada introspection to match.Closes #1904.
Test plan
pnpm -F ensapi typecheckpnpm -F enssdk typecheckpnpm lintpnpm test --project ensapi --project enssdk🤖 Generated with Claude Code