----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32151/#review78081 -----------------------------------------------------------
Ship it! src/common/version.cpp <https://reviews.apache.org/r/32151/#comment126502> Let's include: << " failed to parse Mesos version '" << MESOS_VERSION << "'"; Here and below please! src/common/version.cpp <https://reviews.apache.org/r/32151/#comment126508> Let's just: return version->get().major; Here and below please. src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126510> This should all be Javadoc comments (/** */). src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126485> s/new/New/ src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126486> s/old/Old/ src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126496> implements Comparable src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126511> Javadoc comments please, here and everywhere else! src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126497> @Override src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126500> if (other == null) { throw new IllegalArgumentException("other Version must not be null"); } if (major < other.major) { return -1; } else if (major > other.major) { return 1; } if (minor < other.minor) { return -1; } else if (minor > other.minor) { return 1; } ... src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126509> Let's leave a comment that this is not "equal to and before" but just strictly before, similar down below for 'after' as well. src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in <https://reviews.apache.org/r/32151/#comment126491> Let's make this a RuntimeException instead. Why? This provides similar semantics to if we attempted to call a native method and haven't first called MesosNativeLibrary.load(). - Benjamin Hindman On March 27, 2015, 12:01 a.m., Joris Van Remoortere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32151/ > ----------------------------------------------------------- > > (Updated March 27, 2015, 12:01 a.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Bugs: MESOS-1795 and MESOS-2161 > https://issues.apache.org/jira/browse/MESOS-1795 > https://issues.apache.org/jira/browse/MESOS-2161 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/version.hpp.in 524cebce5d84edac3ef5d0b72e3989f283164809 > src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb > src/common/version.cpp PRE-CREATION > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in > 668647fdfc0e203fcde59263256659ba14e29960 > src/java/jni/org_apache_mesos_MesosNativeLibrary.cpp PRE-CREATION > src/tests/common/version_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/32151/diff/ > > > Testing > ------- > > > Thanks, > > Joris Van Remoortere > >
