Hm.. any reason that unrelated headers were touched and the using statement was removed in this patch?
My concern with mixing unrelated changes within a single patch is that patches become less precise. If one reads the patch there is additional overhead in understanding what is related to the goal of the change and what is not. I know it's a small example here but I see value in being disciplined about this regardless of patch size. The other concern is that the reviewer of this patch has to review these two additional changes: 1. How does the header audit look? Anything else need added or removed? 2. How does the 'using' audit look? Anything else need added or removed? (1) and (2) could be done together in a single patch. As in turns out, the header audit looks like it has a few issues, but I'm guessing the reviewer glossed over it because the point of this change was CHECK_READY :) -<stout/check.hpp> was removed but CHECK_NONE and CHECK_SOME are used -<glog/logging.h> is not present but LOG is used -<stout/nothing.hpp> is absent but Nothing is used -<process/process.hpp> is absent but process::Process / process::wait / process::terminate are used. Then for the 'using' audit, we now avoid pulling in all of the process:: namespace in favor of finer-grained using statements. On Mon, May 2, 2016 at 4:08 AM, <[email protected]> wrote: > Repository: mesos > Updated Branches: > refs/heads/master 78f6101cc -> 4f9040db6 > > > Replaced CHECK with CHECK_READY. > > Also removes some unused header includes. > > Review: https://reviews.apache.org/r/46827/ > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4f9040db > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4f9040db > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4f9040db > > Branch: refs/heads/master > Commit: 4f9040db62294f370f94aaba675f05b1ccf7a310 > Parents: 78f6101 > Author: Neil Conway <[email protected]> > Authored: Mon May 2 13:05:56 2016 +0200 > Committer: Alexander Rukletsov <[email protected]> > Committed: Mon May 2 13:05:56 2016 +0200 > > ---------------------------------------------------------------------- > src/zookeeper/contender.cpp | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4f9040db/src/zookeeper/contender.cpp > ---------------------------------------------------------------------- > diff --git a/src/zookeeper/contender.cpp b/src/zookeeper/contender.cpp > index 4b1cc65..5eb4b58 100644 > --- a/src/zookeeper/contender.cpp > +++ b/src/zookeeper/contender.cpp > @@ -14,26 +14,23 @@ > // See the License for the specific language governing permissions and > // limitations under the License > > -#include <set> > #include <string> > > #include <mesos/zookeeper/contender.hpp> > #include <mesos/zookeeper/detector.hpp> > #include <mesos/zookeeper/group.hpp> > > +#include <process/check.hpp> > #include <process/defer.hpp> > #include <process/dispatch.hpp> > #include <process/future.hpp> > #include <process/id.hpp> > > -#include <stout/check.hpp> > #include <stout/lambda.hpp> > #include <stout/option.hpp> > -#include <stout/some.hpp> > > using namespace process; > > -using std::set; > using std::string; > > namespace zookeeper { > @@ -208,7 +205,7 @@ void LeaderContenderProcess::cancel() > > void LeaderContenderProcess::cancelled(const Future<bool>& result) > { > - CHECK(candidacy.isReady()); > + CHECK_READY(candidacy); > LOG(INFO) << "Membership cancelled: " << candidacy.get().id(); > > // Can be called as a result of either withdraw() or server side > >
