-
Notifications
You must be signed in to change notification settings - Fork 56
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
Greg/alias example #1234
Greg/alias example #1234
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
👍 Looks good to me! Incremental review on 10f4999 in 52 seconds
More details
- Looked at
666
lines of code in18
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. integ-tests/typescript/baml_client/types.ts:321
- Draft comment:
The PR description mentions the addition ofRunFoo2
function and related types, but the code changes show the removal ofFoo2
andFoo3
types andRunFoo2
function. This discrepancy needs clarification. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_FESoynO9lMvEREm4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
❌ Changes requested. Reviewed everything up to 9bd01b8 in 10 minutes and 41 seconds
More details
- Looked at
893
lines of code in25
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_VuTIFCQ9qYCduPk4
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
export interface Foo2 { | ||
bar: number | ||
baz: string | ||
sub?: Foo2 | null | null |
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.
The 'sub' property in the Foo2 interface is redundantly nullable. It should be simplified to sub?: Foo2 | null
. This applies to Foo3 as well.
Important
Adds
RunFoo2
function across clients, new types, refines recursive type handling, and updates unreachable code with context.RunFoo2
function to Python (async_client.py
,sync_client.py
), Ruby (client.rb
), and TypeScript (async_client.ts
,sync_client.ts
) clients.Foo2
andFoo3
types intypes.py
,types.rb
, andtypes.ts
.unreachable!()
withunreachable!("context")
inexpr.rs
,coerce_array.rs
,coerce_optional.rs
, andcoerce_union.rs
.types.rs
.test_constrained_type_alias
intest_runtime.rs
.test_type_alias_with_assert
intest_file_manager.rs
.test_alias_bug
intest_functions.py
.antonio.baml
test file for integration tests.This description was created by for 10f4999. It will automatically update as commits are pushed.