Skip to content
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

stream type constants other than StreamTypeUnary are untyped numerics #485

Closed
jhump opened this issue Mar 24, 2023 · 1 comment · Fixed by #486
Closed

stream type constants other than StreamTypeUnary are untyped numerics #485

jhump opened this issue Mar 24, 2023 · 1 comment · Fixed by #486
Labels
bug Something isn't working

Comments

@jhump
Copy link
Member

jhump commented Mar 24, 2023

Here's the Go code that defines these constants: https://github.com/bufbuild/connect-go/blob/main/connect.go#L48-L53

const (
	StreamTypeUnary  StreamType = 0b00
	StreamTypeClient            = 0b01
	StreamTypeServer            = 0b10
	StreamTypeBidi              = StreamTypeClient | StreamTypeServer
)

I have a very strong suspicion that the last three were meant to be of type StreamType, just like the first one. However, they explicitly state a value but no type, which means they are actually untyped numeric constants. The only time when the type can be omitted and it "inherits" from the previous declaration is when both type and value are omitted.

Sadly, changing this is technically a breaking change, since any code that might be referring to these and expecting them to implicitly be converted to an int (for example) would encounter a compilation error if we "fix" this. However, given that this seems like an obvious accident, maybe there's room for entertaining a fix?

 const (
 	StreamTypeUnary  StreamType = 0b00
-	StreamTypeClient            = 0b01
-	StreamTypeServer            = 0b10
+	StreamTypeClient StreamType = 0b01
+	StreamTypeServer StreamType = 0b10
	StreamTypeBidi              = StreamTypeClient | StreamTypeServer
)
@jhump jhump added the bug Something isn't working label Mar 24, 2023
@akshayjshah
Copy link
Member

Yep, let's fix this. 🤦🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants