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

[V4] Updated EventStreams Byte to SByte to be inline with the spec where Byte was intended to be signed. #3661

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

boblodgett
Copy link
Contributor

Description

Changed EventStreamHeaderType.Byte to EventStreamHeaderType.SByte to be compliant with the specification that a byte is an Int8 which is a signed 8 bit integer not an unsigned 8 bit integer. Updated unit tests and handling of the SByte.

From the SPEC:

struct {
    select (HeaderValueType) {
        case boolean_true:
        case boolean_false:
            struct {};
        case byte:
            int8 value;
        case short:
            int16 value;
        case integer:
            int32 value;
        case long:
            int64 value;
        case byte_array:
            byte data<1..2^15-1>;
        case string:
            utf8 data<1..2^15-1>;
        case timestamp:
            int64 millis_since_epoch;
        case uuid:
            byte value[16];
    } value;
} HeaderValue;

The fixed unit test should have been testing a -47 which it couldn't have done as an unsigned byte.

Motivation and Context

https://sim.amazon.com/issues/DOTNET-7943

Testing

  • Fixed Unit Tests.
  • DryRun: DRY_RUN-a606d350-0953-430b-834d-8565a689965f

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@boblodgett boblodgett merged commit d8cf59a into v4-development Feb 19, 2025
1 check passed
@boblodgett boblodgett deleted the boblod-v4-eventstream-byte-header branch February 19, 2025 17:13
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