You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mahout.apache.org by Dan Filimon <da...@gmail.com> on 2013/04/09 13:31:04 UTC

Code reviews and reviewers

Hi everyone,

Sebastian has been reviewing my code on ReviewBoard [1] for a while now and
I feel bad for always asking him to do it. :)

Is there anyone else who could have a look (I'll also volunteer when you
need a reviewer)?

[1] https://reviews.apache.org/r/10372/

Re: Code reviews and reviewers

Posted by Dan Filimon <da...@gmail.com>.
Awesome! Welcome on board!

I started using it recently and I recall having to make a different account
(well, name it the same :).

The simplest way to check though is to just try your existing one:
https://reviews.apache.org/dashboard/

And... here's a patch to get you started: (shamelessly posting own patch)
https://reviews.apache.org/r/10372/


On Tue, Apr 9, 2013 at 5:26 PM, Saikat Kanjilal <sx...@hotmail.com> wrote:

> If it's ok with other folks I'd love to participate in code reviews.
>  Having never used review board before do we need a separate review board
> account outside of JIRA or confluence?
>
> > From: ted.dunning@gmail.com
> > Date: Tue, 9 Apr 2013 07:13:34 -0700
> > Subject: Re: Code reviews and reviewers
> > To: dev@mahout.apache.org
> >
> > On Tue, Apr 9, 2013 at 5:03 AM, Dan Filimon <dangeorge.filimon@gmail.com
> >wrote:
> >
> > > Thanks Sebastian!
> > >
> > > But let's talk about policy. Wouldn't everyone agree that more code
> reviews
> > > are a good thing?
> > >
> >
> > Absolutely.
> >
> >
> > > Ideally, everyone does it and becomes familiar with the code base.
> > >
> > > So, even though I, for instance wouldn't be able to say much about
> > > something I haven't worked on, like recommendation systems, a sanity
> check
> > > is always good to have.
> > > Also good for having a cohesive style and ensuring the code is easy to
> read
> > > and is adequately documented.
> > >
> >
> > And frankly, it is often not noticed by newcomers that they anybody can
> > participate in code reviews.
> >
> > Also, code reviews are not just about the code being reviewed.  As you
> > read, you are reviewing your own knowledge of the subject at hand.
>  Asking
> > questions in a review is a great way to learn.
> >
> >
> >
> > > Right now, it looks like there aren't that many changes going through
> > > ReviewBoard. Sure, there's JIRA, but for more significant changes?
> > >
> > > Any thoughts on what's best to do?
> > >
> >
> > More reviews are all to the better.
> >
> > The only question is how to convince lurkers to be brave and try
> reviewing.
>
>

RE: Code reviews and reviewers

Posted by Saikat Kanjilal <sx...@hotmail.com>.
If it's ok with other folks I'd love to participate in code reviews.  Having never used review board before do we need a separate review board account outside of JIRA or confluence?

> From: ted.dunning@gmail.com
> Date: Tue, 9 Apr 2013 07:13:34 -0700
> Subject: Re: Code reviews and reviewers
> To: dev@mahout.apache.org
> 
> On Tue, Apr 9, 2013 at 5:03 AM, Dan Filimon <da...@gmail.com>wrote:
> 
> > Thanks Sebastian!
> >
> > But let's talk about policy. Wouldn't everyone agree that more code reviews
> > are a good thing?
> >
> 
> Absolutely.
> 
> 
> > Ideally, everyone does it and becomes familiar with the code base.
> >
> > So, even though I, for instance wouldn't be able to say much about
> > something I haven't worked on, like recommendation systems, a sanity check
> > is always good to have.
> > Also good for having a cohesive style and ensuring the code is easy to read
> > and is adequately documented.
> >
> 
> And frankly, it is often not noticed by newcomers that they anybody can
> participate in code reviews.
> 
> Also, code reviews are not just about the code being reviewed.  As you
> read, you are reviewing your own knowledge of the subject at hand.  Asking
> questions in a review is a great way to learn.
> 
> 
> 
> > Right now, it looks like there aren't that many changes going through
> > ReviewBoard. Sure, there's JIRA, but for more significant changes?
> >
> > Any thoughts on what's best to do?
> >
> 
> More reviews are all to the better.
> 
> The only question is how to convince lurkers to be brave and try reviewing.
 		 	   		  

Re: Code reviews and reviewers

Posted by Ted Dunning <te...@gmail.com>.
On Tue, Apr 9, 2013 at 5:03 AM, Dan Filimon <da...@gmail.com>wrote:

> Thanks Sebastian!
>
> But let's talk about policy. Wouldn't everyone agree that more code reviews
> are a good thing?
>

Absolutely.


> Ideally, everyone does it and becomes familiar with the code base.
>
> So, even though I, for instance wouldn't be able to say much about
> something I haven't worked on, like recommendation systems, a sanity check
> is always good to have.
> Also good for having a cohesive style and ensuring the code is easy to read
> and is adequately documented.
>

And frankly, it is often not noticed by newcomers that they anybody can
participate in code reviews.

Also, code reviews are not just about the code being reviewed.  As you
read, you are reviewing your own knowledge of the subject at hand.  Asking
questions in a review is a great way to learn.



> Right now, it looks like there aren't that many changes going through
> ReviewBoard. Sure, there's JIRA, but for more significant changes?
>
> Any thoughts on what's best to do?
>

More reviews are all to the better.

The only question is how to convince lurkers to be brave and try reviewing.

Re: Code reviews and reviewers

Posted by Suneel Marthi <su...@yahoo.com>.
+3



________________________________
 From: Andrew Musselman <an...@gmail.com>
To: dev@mahout.apache.org 
Sent: Tuesday, April 9, 2013 9:16 AM
Subject: Re: Code reviews and reviewers
 
+1 for code reviews
+1 for Review Board
+1 for unit tests and integration tests


On Tue, Apr 9, 2013 at 5:03 AM, Dan Filimon <da...@gmail.com>wrote:

> Thanks Sebastian!
>
> But let's talk about policy. Wouldn't everyone agree that more code reviews
> are a good thing?
> Ideally, everyone does it and becomes familiar with the code base.
>
> So, even though I, for instance wouldn't be able to say much about
> something I haven't worked on, like recommendation systems, a sanity check
> is always good to have.
> Also good for having a cohesive style and ensuring the code is easy to read
> and is adequately documented.
>
> And, best of all, I get the added bonus of having to read up (at least an
> overview) of how these systems work to provide some more meaningful
> feedback.
>
> Right now, it looks like there aren't that many changes going through
> ReviewBoard. Sure, there's JIRA, but for more significant changes?
>
> Any thoughts on what's best to do?
>
>
>
> On Tue, Apr 9, 2013 at 2:34 PM, Sebastian Schelter
> <ss...@googlemail.com>wrote:
>
> > Dan,
> >
> > it's a pleasure to review your code. Ask me anytime :)
> >
> > On 09.04.2013 13:31, Dan Filimon wrote:
> > > Hi everyone,
> > >
> > > Sebastian has been reviewing my code on ReviewBoard [1] for a while now
> > and
> > > I feel bad for always asking him to do it. :)
> > >
> > > Is there anyone else who could have a look (I'll also volunteer when
> you
> > > need a reviewer)?
> > >
> > > [1] https://reviews.apache.org/r/10372/
> > >
> >
> >
>

Re: Code reviews and reviewers

Posted by Andrew Musselman <an...@gmail.com>.
+1 for code reviews
+1 for Review Board
+1 for unit tests and integration tests


On Tue, Apr 9, 2013 at 5:03 AM, Dan Filimon <da...@gmail.com>wrote:

> Thanks Sebastian!
>
> But let's talk about policy. Wouldn't everyone agree that more code reviews
> are a good thing?
> Ideally, everyone does it and becomes familiar with the code base.
>
> So, even though I, for instance wouldn't be able to say much about
> something I haven't worked on, like recommendation systems, a sanity check
> is always good to have.
> Also good for having a cohesive style and ensuring the code is easy to read
> and is adequately documented.
>
> And, best of all, I get the added bonus of having to read up (at least an
> overview) of how these systems work to provide some more meaningful
> feedback.
>
> Right now, it looks like there aren't that many changes going through
> ReviewBoard. Sure, there's JIRA, but for more significant changes?
>
> Any thoughts on what's best to do?
>
>
>
> On Tue, Apr 9, 2013 at 2:34 PM, Sebastian Schelter
> <ss...@googlemail.com>wrote:
>
> > Dan,
> >
> > it's a pleasure to review your code. Ask me anytime :)
> >
> > On 09.04.2013 13:31, Dan Filimon wrote:
> > > Hi everyone,
> > >
> > > Sebastian has been reviewing my code on ReviewBoard [1] for a while now
> > and
> > > I feel bad for always asking him to do it. :)
> > >
> > > Is there anyone else who could have a look (I'll also volunteer when
> you
> > > need a reviewer)?
> > >
> > > [1] https://reviews.apache.org/r/10372/
> > >
> >
> >
>

Re: Code reviews and reviewers

Posted by Dan Filimon <da...@gmail.com>.
Thanks Sebastian!

But let's talk about policy. Wouldn't everyone agree that more code reviews
are a good thing?
Ideally, everyone does it and becomes familiar with the code base.

So, even though I, for instance wouldn't be able to say much about
something I haven't worked on, like recommendation systems, a sanity check
is always good to have.
Also good for having a cohesive style and ensuring the code is easy to read
and is adequately documented.

And, best of all, I get the added bonus of having to read up (at least an
overview) of how these systems work to provide some more meaningful
feedback.

Right now, it looks like there aren't that many changes going through
ReviewBoard. Sure, there's JIRA, but for more significant changes?

Any thoughts on what's best to do?



On Tue, Apr 9, 2013 at 2:34 PM, Sebastian Schelter
<ss...@googlemail.com>wrote:

> Dan,
>
> it's a pleasure to review your code. Ask me anytime :)
>
> On 09.04.2013 13:31, Dan Filimon wrote:
> > Hi everyone,
> >
> > Sebastian has been reviewing my code on ReviewBoard [1] for a while now
> and
> > I feel bad for always asking him to do it. :)
> >
> > Is there anyone else who could have a look (I'll also volunteer when you
> > need a reviewer)?
> >
> > [1] https://reviews.apache.org/r/10372/
> >
>
>

Re: Code reviews and reviewers

Posted by Sebastian Schelter <ss...@googlemail.com>.
Dan,

it's a pleasure to review your code. Ask me anytime :)

On 09.04.2013 13:31, Dan Filimon wrote:
> Hi everyone,
> 
> Sebastian has been reviewing my code on ReviewBoard [1] for a while now and
> I feel bad for always asking him to do it. :)
> 
> Is there anyone else who could have a look (I'll also volunteer when you
> need a reviewer)?
> 
> [1] https://reviews.apache.org/r/10372/
>