mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 53550: Rename symbols in log.proto to avoid naming collision in win32 API.
Date Thu, 08 Dec 2016 00:30:28 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53550/#review158440
-----------------------------------------------------------


Ship it!




This will not cause any compatibility issues as the enumeration is unchanged.  When these
protobufs are sent over the wire, they are transmitted as byte code (not JSON), which does
not include the name of each enum value.  As such, this will not break between major versions
of the master either.

- Joseph Wu


On Dec. 6, 2016, 6:16 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53550/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The standard win32 headers define a macro, `IGNORE`, which presently
> collides with two uses of the same symbol in log.proto. This causes a
> compile error on Windows.
> 
> In this commit, we rename the symbol in the log.proto files. There are
> two primary reasons for this.
> 
> The first is because the trick we have previously applied to get around
> similar problems (in which we `#undef` the macro, and re-define as a
> global constant) is made somewaht more complex by the fact that the
> log.proto headers are generated by protocol buffers. To implement this
> effectively, we'd have to individually `#undef` at every site we include
> log.pb.h, which is not a huge deal given the number of #include sites,
> but doesn't protect us against future build breaks.
> 
> The second is that the approach of re-naming the symbol in log.proto is
> not very invasive: we need to change a handful of places where the
> system is used, and we never have to think of the issue again. And,
> because it is an internal API, we don't require a major version bump to
> implement the change.
> 
> 
> Diffs
> -----
> 
>   src/log/consensus.cpp 94ddf245c07ccd38d4fe828bc47c98ecf540243f 
>   src/log/coordinator.cpp 72ef0366633b4e4137fd303fa49f9efb166c80c9 
>   src/log/replica.cpp d596e617d4b4eab91245e0a88a3b9479fc75b813 
>   src/messages/log.proto 6a2c26be2afe411e6927cddebcfd9634b2b1b884 
>   src/tests/log_tests.cpp 99954388eb0fad2acde0cedfd7daa3c9379bfb03 
> 
> Diff: https://reviews.apache.org/r/53550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message