Feature Idea - Tags

52 views
Skip to first unread message

Brad Larson

unread,
Aug 19, 2009, 1:26:59 PM8/19/09
to Repo and Gerrit Discussion
I've been trying to think of a generic way to handle a workflow
inefficiency, and want to see what other people think.

In our setup, we have maybe 30-40 developers pushing changes to
gerrit, and 5-7 mergers. We have a minimum of 100 open changes at all
times, and average closer to 3-400. Each change should be peer
reviewed and verified, and then passed on to one of the mergers for a
final sanity check before merging.

Our problem is that it gets tricky to track which changes are in which
state. Each merger is a watcher for at least 30 changes, and they
can't easily tell which ones are ready for a final sanity check vs
which ones are being reworked by developers. A developer has 5-40
changes uploaded at once, so it is not uncommon for a developer to be
unaware that a change is waiting on them to fix some whitespace
violation or something similar. Some developers have a hard time
knowing when a change is ready for peer review.

A workflow system similar to Jira is a possible fix, but it seems like
overkill and too specific.

If we could tag each change, I think that might be a possible
solution. A change ready for peer review is tagged as such. If it is
ready to be merged, the last peer reviewer removes that tag and tags
it as merge. If it gets kicked back for a bug fix, it gets tagged
with the developer's name who needs to upload the new patch. If a
developer wants to check if any changes are waiting on them, they can
search for tags containing their name.

Any thoughts on if this might or might not work out? How are other
teams handling situations like this? I like the tag approach, because
it could be flexed to work in other situations and not just for
workflow. What other solutions should I consider?

Thanks!
Brad

Jean-Baptiste Queru

unread,
Aug 19, 2009, 1:39:42 PM8/19/09
to repo-d...@googlegroups.com
The situation you describe (difficulty for both reviewers and
owners/authors/submitters to know which changes are actually relevant
to them at different points in the process) is something that we're
seeing on the Android Open-Source Project, where we're dealing with a
constant high count of open changes (over 450 the last time I counted)
and where our loose process doesn't help (we don't have full-time
mergers). So, your case isn't unique.

JBQ
--
Jean-Baptiste M. "JBQ" Queru
Software Engineer, Android Open-Source Project, Google.

Questions sent directly to me that have no reason for being private
will likely get ignored or forwarded to a public forum with no further
warning.

Shawn Pearce

unread,
Aug 19, 2009, 1:48:32 PM8/19/09
to repo-d...@googlegroups.com
On Wed, Aug 19, 2009 at 10:26, Brad Larson<bkla...@gmail.com> wrote:
>
> I've been trying to think of a generic way to handle a workflow
> inefficiency, and want to see what other people think.
>
> In our setup, we have maybe 30-40 developers pushing changes to
> gerrit, and 5-7 mergers.  We have a minimum of 100 open changes at all
> times, and average closer to 3-400.  Each change should be peer
> reviewed and verified, and then passed on to one of the mergers for a
> final sanity check before merging.
>
> Our problem is that it gets tricky to track which changes are in which
> state.

Yea, we've noticed the same thing internally at Google. Enough that
Andy opened http://jira.source.android.com/jira/browse/GERRIT-189 to
request for better overview on the My>Changes screen. Unfortunately
nobody has coded anything yet. And as JBQ points out, AOSP also has
this problem.

> A workflow system similar to Jira is a possible fix, but it seems like
> overkill and too specific.

I agree.

> If we could tag each change, I think that might be a possible
> solution.  A change ready for peer review is tagged as such.  If it is
> ready to be merged, the last peer reviewer removes that tag and tags
> it as merge.  If it gets kicked back for a bug fix, it gets tagged
> with the developer's name who needs to upload the new patch.  If a
> developer wants to check if any changes are waiting on them, they can
> search for tags containing their name.

Hmmph. Interesting.

I'd rather not make the user's perform tagging on their own if there
is a reasonably well defined process involved. I've worked with
groups that did workflow routing by manual instructions, I kid you
not, the "instruction book" was many pages long and involved setting
several fields at each "stop" in the process. It was very common for
things to be tagged wrong.

I need to stop dinking around with this LDAP code and move back to the
query stuff I was working on. I'm trying to define a generic query
language, so we could write expressions like:

branch:master project:tools/gerrit verified:+1 codereview:+2 status:open

to find all open changes with both verified +1 and code review +2, so
we can review them. I wonder if we can get the language sufficient
that folks can identify what they need to look at based solely on a
few query expressions, and then we support naming these queries in the
application, to add new menus like "My > To Merge".

But I've only got the query language parser written, and started a
draft of the predicate evaluators. Its still not quite there yet.


On the other side, there is something to be said for "labels". GMail
makes quite heavy use of them, and it actually works fairly well to
organize stuff. When combined with filters, which in Gerrit's case
could be saved query expressions that automatically add or delete a
given label based on the change matching (or not matching), plus
allowing users to manually add/remove these tags on their own, it
might be a pretty powerful, yet simple to use system.

I dunno, I am just tossing things out there. :-)

Brad Larson

unread,
Aug 20, 2009, 6:33:36 PM8/20/09
to Repo and Gerrit Discussion


On Aug 19, 12:48 pm, Shawn Pearce <s...@google.com> wrote:
> On Wed, Aug 19, 2009 at 10:26, Brad Larson<bklar...@gmail.com> wrote:
>
> > I've been trying to think of a generic way to handle a workflow
> > inefficiency, and want to see what other people think.
>
> > In our setup, we have maybe 30-40 developers pushing changes to
> > gerrit, and 5-7 mergers.  We have a minimum of 100 open changes at all
> > times, and average closer to 3-400.  Each change should be peer
> > reviewed and verified, and then passed on to one of the mergers for a
> > final sanity check before merging.
>
> > Our problem is that it gets tricky to track which changes are in which
> > state.
>
> Yea, we've noticed the same thing internally at Google.  Enough that
> Andy openedhttp://jira.source.android.com/jira/browse/GERRIT-189to
> request for better overview on the My>Changes screen.  Unfortunately
> nobody has coded anything yet.  And as JBQ points out, AOSP also has
> this problem.

I've considered Andy's suggesting of showing the verified/code review
status on the list pages. I think it breaks down for deployments that
add other approval categories, which we might do in the future.

>
> > A workflow system similar to Jira is a possible fix, but it seems like
> > overkill and too specific.
>
> I agree.
>
> > If we could tag each change, I think that might be a possible
> > solution.  A change ready for peer review is tagged as such.  If it is
> > ready to be merged, the last peer reviewer removes that tag and tags
> > it as merge.  If it gets kicked back for a bug fix, it gets tagged
> > with the developer's name who needs to upload the new patch.  If a
> > developer wants to check if any changes are waiting on them, they can
> > search for tags containing their name.
>
> Hmmph.  Interesting.
>
> I'd rather not make the user's perform tagging on their own if there
> is a reasonably well defined process involved.  I've worked with
> groups that did workflow routing by manual instructions, I kid you
> not, the "instruction book" was many pages long and involved setting
> several fields at each "stop" in the process.  It was very common for
> things to be tagged wrong.

I haven't used tagging systems much at all. It seems to work OK on
some sites, such as StackOverflow, but I get your point about the
extra work involved and possibility of error.

>
> I need to stop dinking around with this LDAP code and move back to the
> query stuff I was working on.

We use LDAP - Keep up to good work! ;)

> I'm trying to define a generic query
> language, so we could write expressions like:
>
>   branch:master project:tools/gerrit verified:+1 codereview:+2 status:open
>
> to find all open changes with both verified +1 and code review +2, so
> we can review them.  I wonder if we can get the language sufficient
> that folks can identify what they need to look at based solely on a
> few query expressions, and then we support naming these queries in the
> application, to add new menus like "My > To Merge".

I could see saved queries similar to Jira helping out and possibly
solving this problem. I'm not sure if the verified and code review
fields give enough information to determine who needs to work on a
patch next in all cases. It is a good start though, and would be very
useful.

Michael Poole

unread,
Sep 1, 2009, 12:10:51 AM9/1/09
to Repo and Gerrit Discussion
On Aug 20, 6:33 pm, Brad Larson <bklar...@gmail.com> wrote:
> On Aug 19, 12:48 pm, Shawn Pearce <s...@google.com> wrote:
> > I'm trying to define a generic query
> > language, so we could write expressions like:
>
> >   branch:master project:tools/gerrit verified:+1 codereview:+2 status:open
>
> > to find all open changes with both verified +1 and code review +2, so
> > we can review them.  I wonder if we can get the language sufficient
> > that folks can identify what they need to look at based solely on a
> > few query expressions, and then we support naming these queries in the
> > application, to add new menus like "My > To Merge".
>
> I could see saved queries similar to Jira helping out and possibly
> solving this problem.  I'm not sure if the verified and code review
> fields give enough information to determine who needs to work on a
> patch next in all cases.  It is a good start though, and would be very
> useful.

Would a patch implementing the status indicators requested in
GERRIT-189 be interesting in the short term, or is the desire more to
pick a longer-term solution and just go with that?

(I have such a patch done and mostly tested, and am wondering whether
to upload it to review.source.android.com or to stay out of peoples'
hair.)

Michael Poole

Zach Wily

unread,
Sep 1, 2009, 12:29:52 AM9/1/09
to repo-d...@googlegroups.com
> Would a patch implementing the status indicators requested in
> GERRIT-189 be interesting in the short term, or is the desire more to
> pick a longer-term solution and just go with that?
>
> (I have such a patch done and mostly tested, and am wondering whether
> to upload it to review.source.android.com or to stay out of peoples'
> hair.)

I started working on a similar patch myself. :) Please submit it, my
team and I would love something like that.

zach

Reply all
Reply to author
Forward
0 new messages