Skip to content

Conversation

@mugabe
Copy link

@mugabe mugabe commented Nov 26, 2025

No description provided.

Copy link
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

I feel like this is PR is good, but we might need to find a better way to address the issue it tries to fix.

I'm not expecting you to work on a new implementation. I'm sure you can.
I'm not looking for it for now.

I feel like the discussion should continue in the issue you opened rather than here in the code

// Decoder, unmarshals a node into a provided value.

type decoder struct {
ctx context.Context
Copy link
Contributor

Choose a reason for hiding this comment

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

Having a context stored in a struct is an anti pattern.

For me, there is a problem here.

But it might be needed if we don't want a PR that breaks everything and methods signature.

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