mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Park" <mcyp...@gmail.com>
Subject Re: Review Request 34375: Removed use of namespace aliases.
Date Sat, 23 May 2015 08:18:29 GMT


> On May 19, 2015, 2:02 a.m., Ben Mahler wrote:
> > This might be a valid namespace alias use case that we hadn't considered, because
there is no way to be able to write just `http::Response` otherwise, is there?
> > 
> > Seems quite verbose to write process::http everywhere, and on the otherhand just
having `Request` or `Response` seems to miss the context of it being http, thoughts?

> This might be a valid namespace alias use case that we hadn't considered

I actually think we should reconsider the overall approach of disallowing aliases (e.g. type,
namespace).
> because there is no way to be able to write just http::Response otherwise, is there?

We could do so by declaring `using namespace process;` if we wanted to.
> Seems quite verbose to write process::http everywhere and on the otherhand just having
Request or Response seems to miss the context of it being http

This is the entire point of aliases in general, which is why I think we should reconsider
the decision to disallow them.
> thoughts?

>From a offline discussion with BenH, it seems like the decision to disallow type and namespace
aliases to avoid inconsistencies in different files. For example, one person could write `namespace
http = process::http;` and another person could write `namespace phttp = process::http;`.

I think the correct solution is to come up with naming guidelines to give them good and consistent
names, rather than disallowing the feature. For example, we don't disallow functions just
because "someone could possibly give it a horrible name". We instead try to influence people
to give their functions meaningful names in a consistent manner. An example of such rule is
that we write `member()` rather than `getMember()` for our getter functions. We don't simply
disallow getter functions just because people could say them inconsistently.

Anyway, given the current rules, I would push for us to be consistent with no special cases.
Especially since we have much longer names all over in the codebase that we cope with. (e.g.
`mesos::master::allocator::Allocator`, `&master::allocator::MesosAllocatorProcess::deactivateFramework`,
`master::allocator::HierarchicalDRFAllocator`)

I'm definitely interested in starting the discussion around reconsidering the rules and perhaps
formalizing some of the naming rules around aliases, but I also think that that should happen
outside of this review, probably in a JIRA ticket.


- Michael


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


On May 18, 2015, 11:13 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34375/
> -----------------------------------------------------------
> 
> (Updated May 18, 2015, 11:13 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e 
>   src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 
>   src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b 
>   src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 
> 
> Diff: https://reviews.apache.org/r/34375/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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