----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32536/#review80015 -----------------------------------------------------------
Ship it! Thanks for bringing this up Alex - I find it very clear and helpful. - Till Toenshoff On March 26, 2015, 4:10 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32536/ > ----------------------------------------------------------- > > (Updated March 26, 2015, 4:10 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Michael Park, Niklas > Nielsen, and Timothy Chen. > > > Repository: mesos-incubating > > > Description > ------- > > Documents the patterns we use to name variables and function arguments in our > codebase. > > Leading underscores to avoid ambiguity. > ======================================= > > We use this pattern extensively in libprocess, stout and mesos, a few > examples below. > > * `stout/try.hpp:105` > ``` > Try(State _state, T* _t = NULL, const std::string& _message = "") > : state(_state), t(_t), message(_message) {} > ``` > > * `process/http.hpp:480` > ``` > URL(const std::string& _scheme, > const std::string& _domain, > const uint16_t _port = 80, > const std::string& _path = "/", > const hashmap<std::string, std::string>& _query = > (hashmap<std::string, std::string>()), > const Option<std::string>& _fragment = None()) > : scheme(_scheme), > domain(_domain), > port(_port), > path(_path), > query(_query), > fragment(_fragment) {} > ``` > > * `slave/containerizer/linux_launcher.cpp:56` > ``` > LinuxLauncher::LinuxLauncher( > const Flags& _flags, > int _namespaces, > const string& _hierarchy) > : flags(_flags), > namespaces(_namespaces), > hierarchy(_hierarchy) {} > ``` > > Trailing undescores as prime symbols. > ===================================== > > We use this pattern in the code, though not extensively. I would like to see > more pass-by-value instead of creating copies from a variable passed by const > reference. > > * `master.cpp:2942` > ``` > // Create and add the slave id. > SlaveInfo slaveInfo_ = slaveInfo; > slaveInfo_.mutable_id()->CopyFrom(newSlaveId()); > ``` > > * `slave.cpp:4180` > ``` > ExecutorInfo executorInfo_ = executor->info; > Resources resources = executorInfo_.resources(); > resources += taskInfo.resources(); > executorInfo_.mutable_resources()->CopyFrom(resources); > ``` > > * `status_update_manager.cpp:474` > ``` > // Bounded exponential backoff. > Duration duration_ = > std::min(duration * 2, STATUS_UPDATE_RETRY_INTERVAL_MAX); > ``` > > * `containerizer/mesos/containerizer.cpp:109` > ``` > // Modify the flags to include any changes to isolation. > Flags flags_ = flags; > flags_.isolation = isolation; > ``` > > Passing arguments by value. > =========================== > > * `slave.cpp:2480` > ``` > void Slave::statusUpdate(StatusUpdate update, const UPID& pid) > { > ... > // Set the source before forwarding the status update. > update.mutable_status()->set_source( > pid == UPID() ? TaskStatus::SOURCE_SLAVE : TaskStatus::SOURCE_EXECUTOR); > ... > } > ``` > > * `process/metrics/timer.hpp:103` > ``` > static void _time(Time start, Timer that) > { > const Time stop = Clock::now(); > > double value; > > process::internal::acquire(&that.data->lock); > { > that.data->lastValue = T(stop - start).value(); > value = that.data->lastValue.get(); > } > process::internal::release(&that.data->lock); > > that.push(value); > } > ``` > > > Diffs > ----- > > docs/mesos-c++-style-guide.md 439fe12 > > Diff: https://reviews.apache.org/r/32536/diff/ > > > Testing > ------- > > None (not a functional change). > > > Thanks, > > Alexander Rukletsov > >
