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

Deserialize speaker as an int not a string #144

Closed
wants to merge 1 commit into from

Conversation

DamienDeepgram
Copy link
Contributor

Speaker is an int not a string

speaker: 0

@lukeocodes
Copy link
Contributor

@CopperBeardy @ThindalTV chaps can you run your eyes over this. Can you tell me if this is a breaking change for existing implementations?

@ThindalTV
Copy link
Collaborator

Unfortunately, yes.
It is a good catch and something that should definitively be done, but it will break user implementations that use the speaker value and treat it as a string.

@DamienDeepgram
Copy link
Contributor Author

This was added last week (#138) for a specific customer and they reported that it should be an int not a string.

I think it is safe to update given how little time it has been present as prior to the above PR the value was actually missing

@lukeocodes
Copy link
Contributor

This was added last week (#138) for a specific customer and they reported that it should be an int not a string.

I think it is safe to update given how little time it has been present as prior to the above PR the value was actually missing

I appreciate the sentiment, but semver is concrete and breaking changes are major version bumps

@lukeocodes
Copy link
Contributor

Digging further into this. Given the userland workaround to this is int result = Int32.Parse(input); I am going to suggest we include this change in the rearchitecture planned for this SDK in Q4. A string -> int data type change is a breaking change, and requires a major version bump to introduce.

At this point, with a major version in the near future, we'll close this ticket in favour of including it then.

Thanks! (see Discord for the discussion).

@jpvajda
Copy link
Contributor

jpvajda commented Sep 29, 2023

@DamienDeepgram - Just closing the loop in this, We can plan to implement this change in our next Major release of the SDK which is something we are beginning to plan now with our community members. I'm going to open an issue for this to ensure it's accounted for. see #145

@davidvonthenen davidvonthenen deleted the bug-fix-speaker-type branch February 9, 2024 16:26
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.

4 participants