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

Ship it!



docs/mesos-c++-style-guide.md
<https://reviews.apache.org/r/32536/#comment132033>

    Actually, let's prefer trailing underscores for member fields and try and 
evolve away from the "prime symbols" so as not to be confusing.


- Benjamin Hindman


On April 24, 2015, 1:27 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> 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
> -----
> 
>   docs/mesos-c++-style-guide.md de1b93e9f210cd20db2355bd666991339fa4f50b 
> 
> Diff: https://reviews.apache.org/r/32536/diff/
> 
> 
> Testing
> -------
> 
> None (not a functional change).
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to