[
https://issues.apache.org/jira/browse/MESOS-2616?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Till Toenshoff updated MESOS-2616:
----------------------------------
Description:
Our variable naming guide currently is not really explaining use cases for
leading or trailing underscores as found a lot within our codebase.
We should correct that.
The following was copied from the review description for allowing discussions
where needed:
Documents the patterns we use to name variables and function arguments in our
codebase.
h4.Leading underscores to avoid ambiguity.
We use this pattern extensively in libprocess, stout and mesos, a few examples
below.
* stout/try.hpp:105
{noformat}
Try(State _state, T* _t = NULL, const std::string& _message = "")
: state(_state), t(_t), message(_message) {}
{noformat}
* process/http.hpp:480
{noformat}
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) {}
{noformat}
* slave/containerizer/linux_launcher.cpp:56
{noformat}
LinuxLauncher::LinuxLauncher(
const Flags& _flags,
int _namespaces,
const string& _hierarchy)
: flags(_flags),
namespaces(_namespaces),
hierarchy(_hierarchy) {}
{noformat}
h4.Trailing undescores as prime symbols.
We use this pattern in the code, though not extensively. We would like to see
more pass-by-value instead of creating copies from a variable passed by const
reference.
* master.cpp:2942
{noformat}
// Create and add the slave id.
SlaveInfo slaveInfo_ = slaveInfo;
slaveInfo_.mutable_id()->CopyFrom(newSlaveId());
{noformat}
* slave.cpp:4180
{noformat}
ExecutorInfo executorInfo_ = executor->info;
Resources resources = executorInfo_.resources();
resources += taskInfo.resources();
executorInfo_.mutable_resources()->CopyFrom(resources);
{noformat}
* status_update_manager.cpp:474
{noformat}
// Bounded exponential backoff.
Duration duration_ =
std::min(duration * 2, STATUS_UPDATE_RETRY_INTERVAL_MAX);
{noformat}
* containerizer/mesos/containerizer.cpp:109
{noformat}
// Modify the flags to include any changes to isolation.
Flags flags_ = flags;
flags_.isolation = isolation;
{noformat}
h4.Passing arguments by value.
* slave.cpp:2480
{noformat}
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);
...
}
{noformat}
* process/metrics/timer.hpp:103
{noformat}
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);
}
{noformat}
was:
Our variable naming guide currently is not really explaining use cases for
leading or trailing underscores as found a lot within our codebase.
We should correct that.
> Update C++ style guide on variable naming.
> -------------------------------------------
>
> Key: MESOS-2616
> URL: https://issues.apache.org/jira/browse/MESOS-2616
> Project: Mesos
> Issue Type: Documentation
> Reporter: Till Toenshoff
> Assignee: Alexander Rukletsov
> Priority: Minor
>
> Our variable naming guide currently is not really explaining use cases for
> leading or trailing underscores as found a lot within our codebase.
> We should correct that.
> The following was copied from the review description for allowing discussions
> where needed:
> Documents the patterns we use to name variables and function arguments in our
> codebase.
> h4.Leading underscores to avoid ambiguity.
> We use this pattern extensively in libprocess, stout and mesos, a few
> examples below.
> * stout/try.hpp:105
> {noformat}
> Try(State _state, T* _t = NULL, const std::string& _message = "")
> : state(_state), t(_t), message(_message) {}
> {noformat}
> * process/http.hpp:480
> {noformat}
> 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) {}
> {noformat}
> * slave/containerizer/linux_launcher.cpp:56
> {noformat}
> LinuxLauncher::LinuxLauncher(
> const Flags& _flags,
> int _namespaces,
> const string& _hierarchy)
> : flags(_flags),
> namespaces(_namespaces),
> hierarchy(_hierarchy) {}
> {noformat}
> h4.Trailing undescores as prime symbols.
> We use this pattern in the code, though not extensively. We would like to see
> more pass-by-value instead of creating copies from a variable passed by const
> reference.
> * master.cpp:2942
> {noformat}
> // Create and add the slave id.
> SlaveInfo slaveInfo_ = slaveInfo;
> slaveInfo_.mutable_id()->CopyFrom(newSlaveId());
> {noformat}
> * slave.cpp:4180
> {noformat}
> ExecutorInfo executorInfo_ = executor->info;
> Resources resources = executorInfo_.resources();
> resources += taskInfo.resources();
> executorInfo_.mutable_resources()->CopyFrom(resources);
> {noformat}
> * status_update_manager.cpp:474
> {noformat}
> // Bounded exponential backoff.
> Duration duration_ =
> std::min(duration * 2, STATUS_UPDATE_RETRY_INTERVAL_MAX);
> {noformat}
> * containerizer/mesos/containerizer.cpp:109
> {noformat}
> // Modify the flags to include any changes to isolation.
> Flags flags_ = flags;
> flags_.isolation = isolation;
> {noformat}
> h4.Passing arguments by value.
> * slave.cpp:2480
> {noformat}
> 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);
> ...
> }
> {noformat}
> * process/metrics/timer.hpp:103
> {noformat}
> 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);
> }
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
