Re: Patch review process

On Wed, Feb 11, 2015 at 2:04 PM, Steve Loughran wrote: > At the same time, if only 1 person is looking at a part of the codebase & > submitting patches, they have inherently recused themselves from reviewing on > their own patches. Ideally you want >1 committer tracking a topic. That's > someon

Re: Patch review process

On 11 February 2015 at 21:11:25, Chris Douglas (cdoug...@apache.org) wrote: +1; ChrisN's formulation is exactly right. The patch manager can't force (or shame) anyone into caring about your issue. One of the benefits of RTC is that parts of the code with a single m

Re: Patch review process

+1; ChrisN's formulation is exactly right. The patch manager can't force (or shame) anyone into caring about your issue. One of the benefits of RTC is that parts of the code with a single maintainer are exposed. If you can't find collaborators, either (a) this isn't the right community for that mo

Re: Patch review process

I don¹t anticipate a patch manager introducing a new bottleneck. As originally described by Chris D, the role of the patch manager is not to review and commit all patches in an assigned area. Instead, the responsibility is queue management: following up on dormant jiras to make sure progress is m

Re: Patch review process

> How could we speed up? +1 for trying Crucible. We should try whether it's integrated well and it can solve the problem of "splitting discussion". If Crucible solves it, it would be great. About the patch manager, I concern that it can delay reviews if the patch size is too small and the amount

Re: Patch review process

On 9 February 2015 at 21:18:52, Colin P. McCabe (cmcc...@apache.org) wrote: What happened with the Crucible experiment? Did we get a chance to try that out? That would be a great way to speed up patch reviews, and one that is well-integrated with JIRA. I am -1 on Ge

Re: Patch review process

What happened with the Crucible experiment? Did we get a chance to try that out? That would be a great way to speed up patch reviews, and one that is well-integrated with JIRA. I am -1 on Gerrit unless we can find a way to mirror the comments to JIRA. I think splitting the discussion is a far w

Re: Patch review process

On 8 February 2015 at 09:55:42, Karthik Kambatla (ka...@cloudera.com) wrote: On Fri, Feb 6, 2015 at 6:14 PM, Colin P. McCabe wrote: > I think it's healthy to have lots of JIRAs that are "patch available." > It means that there is a lot of interest in the project an

Re: Patch review process

On Fri, Feb 6, 2015 at 6:14 PM, Colin P. McCabe wrote: > I think it's healthy to have lots of JIRAs that are "patch available." > It means that there is a lot of interest in the project and people > want to contribute. It would be unhealthy if JIRAs that really needed > to get in were not getti

Re: Patch review process

On 7 February 2015 at 02:14:39, Colin P. McCabe (cmcc...@apache.org) wrote: I think it's healthy to have lots of JIRAs that are "patch available." It means that there is a lot of interest in the project and people want to contribute. It would be unhealthy if JIRAs that r

Re: Patch review process

I think it's healthy to have lots of JIRAs that are "patch available." It means that there is a lot of interest in the project and people want to contribute. It would be unhealthy if JIRAs that really needed to get in were not getting in. But beyond a few horror stories, that usually doesn't see

Re: Patch review process

I'm thinking it's unhealthy to have over 1000 JIRAs patch available. Reviewers should be more welcome and should review patches from everywhere to increase developers and future reviewers. I'm not completely sure patch managers will make it healthy, however, changing the process (and this disc

Re: Patch review process

+1 to patch managers per component. On Wed, Feb 4, 2015 at 12:29 PM, Allen Wittenauer wrote: > > Is process really the problem? Or, more directly, how does any of > this actually increase the pool beyond the (I’m feeling generous today) 10 > or so committers (never mind PMC) that actua

Re: Patch review process

There are many ways to find reviewers. Look at the set of watchers, email people who work on that component (check git if you're unsure who's been there recently), or even email random committers and ask for leads. Privately ask people why they stopped responding to an issue. Even if an issue has a

Re: Patch review process

Is process really the problem? Or, more directly, how does any of this actually increase the pool beyond the (I’m feeling generous today) 10 or so committers (never mind PMC) that actually review patches that come from outside their employers on a regular basis? To put this in

Re: Patch review process

Release managers are just committers trying to roll releases; it's not an enduring role. A patch manager is just someone helping to track work and direct reviewers to issues. The job doesn't come with a hat. We could look into a badge and gun if that would help. This doesn't require a lot of hand-

Re: Patch review process

The main JIRA dashboard for each project has an Issues tab with useful summary statistics and links to filtered queries, most notably links to unresolved issues grouped by each project sub-component. https://issues.apache.org/jira/browse/HADOOP/?selectedTab=com.atlassian.jir a.jira-projects-plugin

Re: Patch review process

I'm worrying more about the ongoing situation. As a release approaches someone effectively goes full time as the gatekeeper, -for a good release they should be saying "too late!" for most features and "only if it's low risk" to non-critical bug fixes Which means that non-critical stuff don't g

Re: Patch review process

I wonder if this work logically falls under the release manager role. During a release, we generally spend a little bit of time thinking about what new features we added, systems we stabilized, interfaces we changed, etc. etc. This gives us some perspective to look backwards at old JIRAs and eith

Re: Patch review process

+1 on the idea of patch managers. As the patch managers should have good expertise on the specific fields, they are more productive on reviewing the patches and driving the development on the specific fields forward. ~Haohui On Mon, Feb 2, 2015 at 12:55 PM, Chris Nauroth wrote: > I like the ide

Re: Patch review process

I like the idea of patch managers monitoring specific queues of issues, perhaps implemented as a set of jira filters on different values for the component or label fields. Right now, looking at the whole HADOOP backlog is daunting. Using separate filtered review queues could help each reviewer fo

Re: Patch review process

Many projects have unofficial "patch managers": http://producingoss.com/en/share-management.html#patch-manager People who go through outstanding issues, ensuring that each has reached a stable state, or at least a willing reviewer. -C On Mon, Feb 2, 2015 at 3:45 AM, Steve Loughran wrote: > > Gi

Re: Patch review process

Given experience of apache reviews, I don't know how much time to spend on it. I'm curious about Gerrit, but again, if JIRA integration is what is sought, Cruicible sounds better. Returning to other issues in the discussion 1. Improving test times would make a big difference; locally as well a

Re: Patch review process

The fact that reviews.apache.org has ~35k users ( https://reviews.apache.org/users/?page=711 ) that mostly appear to be bots gives me zero confidence in using this tool for anything real. On Jan 30, 2015, at 11:11 AM, Gera Shegalov wrote: > Splitting the conversation via reviewboard and JIRA

Re: Patch review process

Splitting the conversation via reviewboard and JIRA is definitely a problem that we have hit previously [1]. Since reviewboard and probably other tools as well generate emails for each set of comments we could leverage JIRA's functionality [2] to make sure that they are reflected in the JIRA as we

Re: Patch review process

I really do not think it's worth looking at Reviewboard at reviews.apache.org again. We have used it in the past, and it has all the downsides of gerrit and none of the upsides. And some extra downsides of its own. * Splits the conversation into two places * No way to search the split out conve

Re: Patch review process

> > Andrew, can the community build on your distributed pre-commit work to make > it production ready? > > I'm happy to share it if someone is willing to take it across the finish line. I think it'd be about two weeks of work full-time. I've already cleaned it up some, but it still requires checkin

Re: Patch review process

Filed INFRA-9071 to update reviews.apache.org working with the hadoop git repo. Let's see if that works out. Looks like Crucible is no go from infra - https://issues.apache.org/jira/browse/INFRA-8430. On Tue, Jan 27, 2015 at 2:50 AM, Steve Loughran wrote: > I'd be +1 on trying reviews.apache.or

Re: Patch review process

I'd be +1 on trying reviews.apache.org on a JIRA which 1. had multiple distributed people working on it 2. had some tangible code needing reviewing 3. was of limited enough size/duration that we'd see how well it worked do that, get feedback from the participants and repeat until we're h

Re: Patch review process

As popular as the github workflow is, there are some things I'm not a big fan of. For one, the review tends to cause a flurry of separate notifications as opposed to a single atomic set of comments. On Mon, Jan 26, 2015 at 6:17 PM, Arpit Agarwal wrote: > I think someone mentioned this earlier -

Re: Patch review process

I think someone mentioned this earlier - the concern was keeping the review comments searchable from one location, ideally within Jira. On Mon, Jan 26, 2015 at 5:52 PM, Ravi Prakash wrote: > Just out of left field: Since we already migrated to git (Thanks everyone > for that effort) should we t

Re: Patch review process

Just out of left field: Since we already migrated to git (Thanks everyone for that effort) should we try using github's UI? Isn't that how the majority of the rest of the world started doing code reviews? On Monday, January 26, 2015 3:57 PM, Arpit Agarwal wrote: Thanks for that da

Re: Patch review process

Thanks for that data point Chris. It looks like reviews.apache.org no longer works. hadoop-hdfs-git may be pointing to an outdated repository. I'll file a ticket with Infra. On Mon, Jan 26, 2015 at 2:41 PM, Chris Nauroth wrote: > reviews.apache.org is backed by Review Board, and I've had a very

Re: Patch review process

reviews.apache.org is backed by Review Board, and I've had a very positive experience with that tool on other projects. HADOOP-9629 is a Hadoop patch where we decided to go ahead and use it, and I think it helped. AFAIK, there is no rule against using it in Hadoop, but it does have the side effec

Re: Patch review process

IMO the number one improvement would be a web-based review tool. We could evaluate Atlassian Crucible since it claims to integrate well with Jira. I have not tried https://reviews.apache.org/r/new/. Some easy improvements that were also raised by others on the private list: - Encourage contributor

Re: Patch review process

I'm moving this all to common-dev@ as general is more where announcements go than anything else. I think too many patches are falling by the wayside. It takes a lot of time and effort to get patches in, small patches that aren't viewed as critical tend to atrophy: lost in "patch-availalble" state,

Re: Patch review process

Let's move this over to common-dev@, general@ is normally used for project announcements rather than discussion topics. I'd like to summarize a few things mentioned on the private@ thread, related to streamlining the code submission process. - Gerrit was brought up again, as it has in the past, a