You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2016/02/22 08:50:51 UTC
Review Request 43826: Added 'Synchronized Statement in Mesos' blog
post.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/
-----------------------------------------------------------
Review request for mesos, Joerg Schad and Neil Conway.
Repository: mesos
Description
-------
See summary.
Diffs
-----
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
Diff: https://reviews.apache.org/r/43826/diff/
Testing
-------
Locally rendered with `rake && rake dev` from `/site`
Thanks,
Michael Park
Re: Review Request 43826: Added 'Synchronized Statement in Mesos'
blog post.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/#review120126
-----------------------------------------------------------
Fix it, then Ship it!
LGTM. I am still convinced that using scope deliberately is a core tool when programming in C++, and starting with a motivating example from Java seems weird, but what you wrote here reads well and makes a lot of sense.
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 49)
<https://reviews.apache.org/r/43826/#comment181507>
s/^w/W/
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 97)
<https://reviews.apache.org/r/43826/#comment181505>
s/constrast/contrast/
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (lines 302 - 303)
<https://reviews.apache.org/r/43826/#comment181506>
s/evalutes/evaluates/g
- Benjamin Bannier
On Feb. 22, 2016, 8:50 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 8:50 a.m.)
>
>
> Review request for mesos, Joerg Schad and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43826/diff/
>
>
> Testing
> -------
>
> Locally rendered with `rake && rake dev` from `/site`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 43826: Added 'Synchronized Statement in Mesos'
blog post.
Posted by Neil Conway <ne...@gmail.com>.
> On Feb. 22, 2016, 7:30 p.m., Neil Conway wrote:
> > site/source/blog/2016-02-16-synchronized-statements-in-mesos.md, line 286
> > <https://reviews.apache.org/r/43826/diff/1/?file=1264057#file1264057line286>
> >
> > "Remembering all of these rules is simply too complicated, so we opt to use a more clever solution."
>
> Benjamin Bannier wrote:
> Stongly disagree: *clever* carries a negative connotation, while *solve properly* is positive.
"So we opt to solve the problem in a different manner." is fine with me also -- I was more suggesting the sentence flow be rephrased.
> On Feb. 22, 2016, 7:30 p.m., Neil Conway wrote:
> > site/source/blog/2016-02-16-synchronized-statements-in-mesos.md, line 292
> > <https://reviews.apache.org/r/43826/diff/1/?file=1264057#file1264057line292>
> >
> > The usage "a control flow" is a bit unusual (at least to me). What about "a program fragment that is easier for the compiler to reason about"?
>
> Benjamin Bannier wrote:
> IamNotAComputerScientist, but I believe this is standard compiler lingo (cf. *control flow graph*).
"control flow" is certainly standard, I just haven't seen it usually referred to as "a control flow" (in CFG, the object is a "graph"; a CFG is a type of graph).
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/#review120166
-----------------------------------------------------------
On Feb. 22, 2016, 7:48 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 7:48 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joerg Schad, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43826/diff/
>
>
> Testing
> -------
>
> Locally rendered with `rake && rake dev` from `/site`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 43826: Added 'Synchronized Statement in Mesos'
blog post.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 22, 2016, 8:30 p.m., Neil Conway wrote:
> > site/source/blog/2016-02-16-synchronized-statements-in-mesos.md, line 286
> > <https://reviews.apache.org/r/43826/diff/1/?file=1264057#file1264057line286>
> >
> > "Remembering all of these rules is simply too complicated, so we opt to use a more clever solution."
Stongly disagree: *clever* carries a negative connotation, while *solve properly* is positive.
> On Feb. 22, 2016, 8:30 p.m., Neil Conway wrote:
> > site/source/blog/2016-02-16-synchronized-statements-in-mesos.md, line 292
> > <https://reviews.apache.org/r/43826/diff/1/?file=1264057#file1264057line292>
> >
> > The usage "a control flow" is a bit unusual (at least to me). What about "a program fragment that is easier for the compiler to reason about"?
IamNotAComputerScientist, but I believe this is standard compiler lingo (cf. *control flow graph*).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/#review120166
-----------------------------------------------------------
On Feb. 22, 2016, 8:48 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 8:48 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joerg Schad, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43826/diff/
>
>
> Testing
> -------
>
> Locally rendered with `rake && rake dev` from `/site`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 43826: Added 'Synchronized Statement in Mesos'
blog post.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/#review120166
-----------------------------------------------------------
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 15)
<https://reviews.apache.org/r/43826/#comment181565>
"Java language feature" -- or in any case I'd probably drop the hyphen.
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 35)
<https://reviews.apache.org/r/43826/#comment181566>
"class and"
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 42)
<https://reviews.apache.org/r/43826/#comment181579>
I'm a little confused by the semantics of the proposed `if` statement -- you're saying the enclosing block only executes if `condition` is true? Syntactically it seems weird as well.
How about
```
{
if_guard my_if(condition);
// Block only executes if `condition` is true
}
```
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 54)
<https://reviews.apache.org/r/43826/#comment181593>
No comma
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 57)
<https://reviews.apache.org/r/43826/#comment181580>
Remove the comma.
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 64)
<https://reviews.apache.org/r/43826/#comment181583>
Seems a little unclear what you're referring to by "the patterns involving other RAII classes such as `std::vector`"
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 115)
<https://reviews.apache.org/r/43826/#comment181601>
"scope and then destroyed"
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 121)
<https://reviews.apache.org/r/43826/#comment181602>
Remove comma.
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 276)
<https://reviews.apache.org/r/43826/#comment181610>
"If"
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 286)
<https://reviews.apache.org/r/43826/#comment181611>
"Remembering all of these rules is simply too complicated, so we opt to use a more clever solution."
site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 292)
<https://reviews.apache.org/r/43826/#comment181613>
The usage "a control flow" is a bit unusual (at least to me). What about "a program fragment that is easier for the compiler to reason about"?
- Neil Conway
On Feb. 22, 2016, 7:50 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 7:50 a.m.)
>
>
> Review request for mesos, Joerg Schad and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43826/diff/
>
>
> Testing
> -------
>
> Locally rendered with `rake && rake dev` from `/site`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 43826: Added 'Synchronized Statement in Mesos'
blog post.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43826/#review120162
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43826]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 22, 2016, 7:50 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 7:50 a.m.)
>
>
> Review request for mesos, Joerg Schad and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> site/source/blog/2016-02-16-synchronized-statements-in-mesos.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43826/diff/
>
>
> Testing
> -------
>
> Locally rendered with `rake && rake dev` from `/site`
>
>
> Thanks,
>
> Michael Park
>
>