Skip to content

Use a name derived from the context to name NewProtocols#2

Merged
msullivan merged 3 commits intomainfrom
better-names
Oct 8, 2025
Merged

Use a name derived from the context to name NewProtocols#2
msullivan merged 3 commits intomainfrom
better-names

Conversation

@msullivan
Copy link
Collaborator

No description provided.

@msullivan msullivan requested a review from 1st1 October 8, 2025 20:04
@msullivan msullivan changed the title Use the calling frame's name to produce a name for the NewProtocol Use a name derived from the context to name NewProtocols Oct 8, 2025
name = "NewProtocol"

# If the type evaluation context
ctx = type_eval._current_context.get()
Copy link
Member

Choose a reason for hiding this comment

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

ctx must always be set; I recommend we add a function to get the current context that will always return a non-optional context ref

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Roger, and we are OK with NewProtocolMeta only getting called from our evaluator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha, looks like we aren't setting a context during eval_call

Copy link
Member

@1st1 1st1 left a comment

Choose a reason for hiding this comment

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

Good, aside from one nit

@msullivan msullivan merged commit d191824 into main Oct 8, 2025
1 check passed
@msullivan msullivan deleted the better-names branch October 8, 2025 21:46
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.

2 participants