[chore][pkg/stanza] refactor: introduce emit.Token struct #36260
Conversation
This makes the code clearer by encapsulating the token's body and attributes in a single structure. It should make future change clearer when the emit callback will be changed to accept a collection of tokens as opposed to a single token. The Sink type could use some refactoring as well, but I'm not doing it here to keep the changes to the minimum for clarity and ease of code review.
| @@ -7,4 +7,16 @@ import ( | |||
| "context" | |||
| ) | |||
|
|
|||
| type Callback func(ctx context.Context, token []byte, attrs map[string]any) error | |||
| type Callback func(ctx context.Context, token Token) error | |||
There was a problem hiding this comment.
I considered if the token parameter should be a pointer *Token, but since the Token struct only contains two "reference type" members, a slice and a map, I thought it wouldn't help much to create a pointer to a struct that contains two pointers? 🤔
Please let me know if I'm missing anything. My Go language skills are still limited.
There was a problem hiding this comment.
Seems reasonable to me. This is technically a breaking change, but I think it obscure enough code that we should only note it as a changelog.
There was a problem hiding this comment.
Right, thanks for pointing this out. I'll add an API changelog entry for this.
|
I only ran Stanza tests before pushing the code, but need to also change other parts of Contrib code 🤦 Changing this to draft until I update the other codes. |
But skip the generated files
This makes the code clearer by encapsulating the token's body and attributes in a single structure. It should make future change clearer when the emit callback will be changed to accept a collection of tokens as opposed to a single token.
The Sink type could use some refactoring as well, but I'm not doing it here to keep the changes to the minimum for clarity and ease of code review.