Work around Windows min/max bug.

clalancette commented 4 years ago

The comments in the code explain why we need this.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Some upcoming changes to pluginlib are going to cause us to #include <windows.h>, which is why this is now needed to build on Windows.

One other note: I believe that there is an alternative way to solve this, which is to use MSVC push_macro/pop_macro. That is, we'd do something like:

#ifdef _WIN32
#pragma push_macro("max")
#undef max
#endif

// Use std::numeric_limits<size_t>::max();

#ifdef _WIN32
#pragma pop_macro("max")
#endif

Both are kind of ugly, so I don't have a strong opinion one way or another.

clalancette commented 4 years ago

subjectively I like #undef max more than an empty macro, but I don't have any non-subjective reasons for choosing one over the other.

On balance, after looking at it more, I agree with you. I'm going to change to the other style and then run CI with that.

clalancette commented 4 years ago

CI:

clalancette commented 4 years ago

New CI:

sloretz commented 4 years ago

FYI @clalancette windows CI looks like a java traceback, twice. 3rd attempt in progress:

clalancette commented 4 years ago

Windows finally finished in this job:

With green CI, and approval, I'm going to go ahead and merge this. Thanks for the review!

ros2 / urdf