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

thrift: clean up enum value assignment #88

Closed
wants to merge 1 commit into from

Conversation

djwatson
Copy link

Summary:
Clean up how enum values are handled if an integer value is not
explicitly specified in the thrift file.

For example, the following used to be a compile error, but
works now:

  enum MyEnum {
    SOMEVALUE
  }
  struct MyStruct {
    1: MyEnum e = SOMEVALUE
  }

This change also cleans up some of the error handling with out-of-range
values.  Previously thrift simply issued a warning for enum values that
didn't fit in an i32, but serialized them as i32 anyway.  Now
out-of-range enum values result in a compile failure.

Test Plan:
Included a new unit test to verify the assignment of enum values.  I
also verified that g++ makes the same enum value assignments when
compiling these enums as C++ code.

    Summary:
    Clean up how enum values are handled if an integer value is not
    explicitly specified in the thrift file.

    For example, the following used to be a compile error, but
    works now:

      enum MyEnum {
        SOMEVALUE
      }
      struct MyStruct {
        1: MyEnum e = SOMEVALUE
      }

    This change also cleans up some of the error handling with out-of-range
    values.  Previously thrift simply issued a warning for enum values that
    didn't fit in an i32, but serialized them as i32 anyway.  Now
    out-of-range enum values result in a compile failure.

    Test Plan:
    Included a new unit test to verify the assignment of enum values.  I
    also verified that g++ makes the same enum value assignments when
    compiling these enums as C++ code.
@Jens-G
Copy link
Member

Jens-G commented Apr 2, 2014

There is no ticket yet, right?

@djwatson
Copy link
Author

djwatson commented Apr 2, 2014

not yet

@bufferoverflow
Copy link
Contributor

I made this: https://issues.apache.org/jira/browse/THRIFT-2513

testing...

@asfbot
Copy link

asfbot commented May 30, 2014

Build triggered. Test FAILed.

@asfgit asfgit closed this in ae0b22c Sep 4, 2014
allengeorge pushed a commit to allengeorge/thrift that referenced this pull request Jan 1, 2017
Patch: Dave Watson

This closes apache#88

Summary:
Clean up how enum values are handled if an integer value is not
explicitly specified in the thrift file.

For example, the following used to be a compile error, but
works now:

  enum MyEnum {
	SOMEVALUE
  }
  struct MyStruct {
	1: MyEnum e = SOMEVALUE
  }

This change also cleans up some of the error handling with out-of-range
values.  Previously thrift simply issued a warning for enum values that
didn't fit in an i32, but serialized them as i32 anyway.  Now
out-of-range enum values result in a compile failure.

Test Plan:
Included a new unit test to verify the assignment of enum values.  I
also verified that g++ makes the same enum value assignments when
compiling these enums as C++ code.
iredelmeier pushed a commit to lightstep/thrift that referenced this pull request Oct 23, 2018
* adding initial ginkgo tests

* 💄

* fixed race condition in test

* simplifying tests and adding ginkgo to makefile

* moved tests to testing package

* grpc server connection test, work towards thrift server connection test

* 💄

* adding mock generation for thrift

* adding fakes

* added more recorder tests

* rewrote options_test.go in new style

* moved faking to client side

* converted carrier_test

* converted span_test

* renamed recorder_test to tracer_test

* added custom matcher for KeyValue checking

* 💄

* renamed confusing duplicate config struct

* refactored basictracer/tracer.go to tracer.go

* copied as much as possible from basictracer/ into ./ the grpc code still dependts on basictracer/raw.go and basictracer/span.go

* moved everything in basictracer and thrift_rpc into the top level

* added thrift tests

* seperated report loop logic and transport logic

* added back thrift metrics

* cleaned up some redundant fields

* refactor

* fixed maxlogkeylen bug in thrift recorder

* removed unused options, refactor

* fixing benchmark tests

* 💄

* fixed thrift guid bug

* dissolved Recorder into Tracer

* fixed import in test

* Fix ginkgo call in make test

* Final cleanup.

* MinReportingPeriod option and Changelog update.

* Faster tests.

* fixing possible null pointer deref

* fixing MinReportingPeriod bug

* removed calls to make where able and 💄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants