Re: Semantics of LGTM

2015-01-19 Thread Prashant Sharma
Ousterhout k...@eecs.berkeley.edu wrote: +1 to Patrick's proposal of strong LGTM semantics. On past projects, I've heard the semantics of LGTM expressed as I've looked at this thoroughly and take as much ownership as if I wrote the patch myself. My understanding is that this is the level

Re: Semantics of LGTM

2015-01-19 Thread Patrick Wendell
k...@eecs.berkeley.edu wrote: +1 to Patrick's proposal of strong LGTM semantics. On past projects, I've heard the semantics of LGTM expressed as I've looked at this thoroughly and take as much ownership as if I wrote the patch myself. My understanding is that this is the level

Re: Semantics of LGTM

2015-01-19 Thread Patrick Wendell
. or +1 on the direction The build part looks good to me ... On Sat, Jan 17, 2015 at 8:49 PM, Kay Ousterhout k...@eecs.berkeley.edu wrote: +1 to Patrick's proposal of strong LGTM semantics. On past projects, I've heard the semantics of LGTM expressed as I've looked

Re: Semantics of LGTM

2015-01-18 Thread Reynold Xin
k...@eecs.berkeley.edu wrote: +1 to Patrick's proposal of strong LGTM semantics. On past projects, I've heard the semantics of LGTM expressed as I've looked at this thoroughly and take as much ownership as if I wrote the patch myself. My understanding is that this is the level of review we

Re: Semantics of LGTM

2015-01-17 Thread Matei Zaharia
subsequent reviewers need to reason about what the person meant by saying LGTM. In contrast, having strong semantics around LGTM can help streamline reviews a lot, especially as reviewers get more experienced and gain trust from the comittership. There are several easy ways to give a more

Re: Semantics of LGTM

2015-01-17 Thread sandy . ryza
any show-stopper problems with this patch. The issue with the latter is that it ultimately erodes the significance of LGTM, since subsequent reviewers need to reason about what the person meant by saying LGTM. In contrast, having strong semantics around LGTM can help streamline reviews a lot

Re: Semantics of LGTM

2015-01-17 Thread Aaron Davidson
problems with this patch. The issue with the latter is that it ultimately erodes the significance of LGTM, since subsequent reviewers need to reason about what the person meant by saying LGTM. In contrast, having strong semantics around LGTM can help streamline reviews a lot, especially

Re: Semantics of LGTM

2015-01-17 Thread Kay Ousterhout
+1 to Patrick's proposal of strong LGTM semantics. On past projects, I've heard the semantics of LGTM expressed as I've looked at this thoroughly and take as much ownership as if I wrote the patch myself. My understanding is that this is the level of review we expect for all patches

Re: Semantics of LGTM

2015-01-17 Thread Reza Zadeh
, having strong semantics around LGTM can help streamline reviews a lot, especially as reviewers get more experienced and gain trust from the comittership. There are several easy ways to give a more limited endorsement of a patch: - I'm not familiar with this code, but style, etc look good

Re: Semantics of LGTM

2015-01-17 Thread sandy . ryza
. Nonetheless, I'd prefer to stick with the stronger LGTM semantics I proposed originally (unlike the one Sandy proposed, e.g.). This is what I've seen every project using the LGTM convention do (Google, and some open source projects such as Impala) to indicate technical sign-off. - Patrick

Semantics of LGTM

2015-01-17 Thread Patrick Wendell
problems with this patch. The issue with the latter is that it ultimately erodes the significance of LGTM, since subsequent reviewers need to reason about what the person meant by saying LGTM. In contrast, having strong semantics around LGTM can help streamline reviews a lot, especially as reviewers