On Jan 20, 2011, at 10:53 PM, Namit Jain wrote:
>
> As good practice, also copy important review comments in the jira, so that
> they are searchable and give enough context to a new contributor.
The old review board instance did this automatically. Todd has an INFRA JIRA
open to get this for
I was thinking about it. Let us do the following:
For big patches (a big patch is one which involves changing more than
10 files - those files can be test/code changes or even generated files),
reviewBoard is mandatory.
For smaller patches, reviewBoard is optional. However, if any reviewer
asks f
On Thu, Jan 20, 2011 at 5:56 PM, Namit Jain wrote:
> I agree on the problem of comments.
> For a new reviewer, it makes the process more painful.
>
> But, I think, the issues are not major, and we should
> decide one approach or another.
>
> Even for option 2., we can make it mandatory for the con
I agree on the problem of comments.
For a new reviewer, it makes the process more painful.
But, I think, the issues are not major, and we should
decide one approach or another.
Even for option 2., we can make it mandatory for the contributor
to submit a reviewboard request if the reviewer asks fo
I would argue that how to do the review has nothing to do with code
quality. No review board has no connection with low quality code.
The review board just provides a limited view of the diff's context.
So if the review is familiar with the diff's context and is confident
with the patch, it is his
On Thu, Jan 20, 2011 at 12:12 PM, John Sichi wrote:
> +1 on option 1. This is standard operating practice (for all code changes,
> no exceptions) at Facebook, Google, and many other companies that care about
> code quality.
>
> (The latest HBase wiki makes an exception for patches that only invo
+1 on option 1. This is standard operating practice (for all code changes, no
exceptions) at Facebook, Google, and many other companies that care about code
quality.
(The latest HBase wiki makes an exception for patches that only involve one
changed+unreviewed file, but I think that creates
The system that we have in place right now places all of the burden on the
reviewer. If you want to look at a patch you have to download it, apply it
to a clean workspace, view it using the diff viewer of your choice, and then
copy your comments back to JIRA along with line numbers and code fragmen
+1 for option 2.
In general, we as a community should be nice to all contributors, and
should avoid doing things that make contributors not comfortable, even
that requires some work from committers. Sometimes it is especially
true for new contributors, like we need to be more patience for new
peop
It would be good to have a policy for submitting a new patch for review.
If the patch is small, usually it is pretty easy to review.But, if it large,
a GUI like reviewboard (https://reviews.apache.org) makes it easy.
So, going forward, I would like to propose either of the following.
1. All pa
10 matches
Mail list logo
