Re: [teampractices] Code review social norms

2016-03-15 Thread Rob Lanphier
On Tue, Mar 15, 2016 at 8:03 PM, Mukunda Modell wrote: > On Tue, Mar 15, 2016 at 6:48 PM, Kevin Smith wrote: > >> I would mention that in some cases, I would prefer to accept the commit >> as is, and then perform minor refactoring, such as changing a name, fixing >> a typo, or rearranging the co

Re: [teampractices] Code review social norms

2016-03-15 Thread Mukunda Modell
Tweaking the code during merge is actually something differential / arcanist already supports. It's amending someone else's revision (outside of the merge process) that is currently prevented in arcanist. That's why I wrote https://secure.phabricator.com/D15468 and proposed it to upstream. I inadve

Re: [teampractices] Code review social norms

2016-03-15 Thread Mukunda Modell
On Tue, Mar 15, 2016 at 6:48 PM, Kevin Smith wrote: > > I would mention that in some cases, I would prefer to accept the commit as > is, and then perform minor refactoring, such as changing a name, fixing a > typo, or rearranging the code. Not only does that clearly separate > authorship, but it

Re: [teampractices] Code review social norms

2016-03-15 Thread Greg Grossmeier
> > > I fully agree with this. It troubles me to openly and strongly disagree > > > with Evan, because I have so much respect for him as an upstream BDFL. > > We > > > have a lot to learn from him when it comes to being a healthy upstream. > > > > > > That said, there is a very unhealthy contrari

Re: [teampractices] Code review social norms

2016-03-15 Thread Rob Lanphier
On Tue, Mar 15, 2016 at 11:26 AM, Greg Grossmeier wrote: > > > On Mon, Mar 14, 2016 at 12:40 PM, James Forrester < > jforres...@wikimedia.org> > > wrote: > > > > > On 14 March 2016 at 19:15, Greg Grossmeier wrote: > > > > > >> Backgroun: > > >> * This started as this task in our Phab: > > >>

Re: [teampractices] Patch review culture of Wikimedia teams

2016-03-15 Thread Kevin Smith
I'll speak for myself, while boldly/foolishly projecting some guesses onto the rest of TPG as well. TPG generally focuses on process and inter-personal issues, and doesn't get into the tech itself. So our focus tends to be at the phab task level, rather than at the gerrit patch level. I'm happy to

Re: [teampractices] Code review social norms

2016-03-15 Thread Kevin Smith
There are lots of different cases here, as well as existing norms, and also preferences. I would mention that in some cases, I would prefer to accept the commit as is, and then perform minor refactoring, such as changing a name, fixing a typo, or rearranging the code. Not only does that clearly se

Re: [teampractices] Code review social norms

2016-03-15 Thread Federico Leva (Nemo)
https://secure.phabricator.com/T10584#164843 to me sounds like a big pile of logical fallacies. Nemo ___ teampractices mailing list teampractices@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/teampractices

Re: [teampractices] Code review social norms

2016-03-15 Thread Greg Grossmeier
> On Mon, Mar 14, 2016 at 12:40 PM, James Forrester > wrote: > > > On 14 March 2016 at 19:15, Greg Grossmeier wrote: > > > >> Backgroun: > >> * This started as this task in our Phab: > >> https://phabricator.wikimedia.org/T121751 "Document best practices to > >> amend a change written by an

Re: [teampractices] Code review social norms

2016-03-15 Thread Rob Lanphier
On Mon, Mar 14, 2016 at 12:40 PM, James Forrester wrote: > On 14 March 2016 at 19:15, Greg Grossmeier wrote: > >> Backgroun: >> * This started as this task in our Phab: >> https://phabricator.wikimedia.org/T121751 "Document best practices to >> amend a change written by another contributor"