mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 32536: Updated variable naming style.
Date Fri, 24 Apr 2015 13:27:25 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32536/
-----------------------------------------------------------

(Updated April 24, 2015, 1:27 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Michael Park, Niklas Nielsen, and
Timothy Chen.


Changes
-------

Replaced <pre> tag with backticks.


Bugs: MESOS-2616
    https://issues.apache.org/jira/browse/MESOS-2616


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 (updated)
-----

  docs/mesos-c++-style-guide.md de1b93e9f210cd20db2355bd666991339fa4f50b 

Diff: https://reviews.apache.org/r/32536/diff/


Testing
-------

None (not a functional change).


Thanks,

Alexander Rukletsov


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message