Skip to content

Conversation

@kiranandcode
Copy link
Contributor

Leaving making it into an operation into another PR. Closes #516

@kiranandcode kiranandcode changed the title update enocding_instructions to take in lexical context as a parameter update encoding_instructions to take in lexical context as a parameter Jan 28, 2026
@kiranandcode kiranandcode requested review from eb8680 and jfeser and removed request for jfeser January 28, 2026 16:24
Copy link
Contributor

@eb8680 eb8680 left a comment

Choose a reason for hiding this comment

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

Can you suggest and add some unit tests?

@kiranandcode
Copy link
Contributor Author

oh, yep will do!

@eb8680
Copy link
Contributor

eb8680 commented Jan 28, 2026

After reflecting on #519 I wonder if @jfeser's other idea to fold this information into Encodable.t rather than making it a separate piece of Encodable is the more sensible approach.

@kiranandcode
Copy link
Contributor Author

After reflecting on #519 I wonder if @jfeser's other idea to fold this information into Encodable.t rather than making it a separate piece of Encodable is the more sensible approach.

could you give a code snippet to clarify? I'm still not entirely sure I have a good idea of what this idea is. As in t should be a type that supports .encoding_instructions?

@kiranandcode
Copy link
Contributor Author

Subsumed by #521

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.

encoding_instructions should take lexical context

2 participants