[core-workflow] Tracker workflow proposal
R. David Murray
rdmurray at bitdance.com
Mon Apr 21 18:04:33 CEST 2014
OK, wall of text time. Sorry about this :)
CPython Tracker Workflow
========================
Framing the Discussion
----------------------
This document is intended as a starting point for discussion. I won't
be offended if we throw it out and start over. Even if you like the
general idea, I'm sure I've made some mistakes and have not managed to
correctly think through all the issues.
I also realize that what I'm talking about here is a non-trivial amount
of work. But if we want to have a 'best in class' workflow infrastructure
(and as Nick has said, we *need* one), we are looking at a non-trivial
amount of work no mater how we achieve it. Discussions of alternate
ways to achieve the same effect are welcome, but I'm betting that in
the long run we really do want to tailor this stuff to *our* workflow,
rather than try to shoehorn ourselves into someone else's workflow.
Which doesn't mean we shouldn't leverage existing tools (like Roundup :),
but does mean there's a lot of tailoring to do to get this right.
That said, feedback from people who have actual experience with tools
that support similar workflows is valuable. The ones I've worked with
were not like what I'm trying to create here, they were either like
what we have now (bug report systems with little to no workflow support)
or pure peer-review systems (that is, there were no non-committers).
I'm volunteering to be the coordinator for the work, and I'm also
volunteering to do as much of it as necessary. That is, I'm planning
to make this the focus of most of the "python time" I have available.
I think that there is a way to implement this workflow or whatever
workflow we decide on as a "new interface" while keeping the existing
interface available, allowing us to test and refine (like, maybe do some
usability testing? :). Roundup has a way to dynamically specify which
page template is used to render a particular page type, and I think we
can leverage that to have two parallel UIs. I could be wrong, though,
in which case we'd need to set up a test tracker instead, which I can do.
(Assuming it does work, there might need to be a bit of glue code to
keep things in sync. And there would need to be some changes in the
data values available in the current UI, but I don't think that would
be a bad idea anyway.)
The scope of this discussion is the workflow for an "issue", which
currently means an entry created by someone in the bug tracker at
bugs.python.org, and goes from the creation of the issue to the resolution
of the issue, which can but does not necessarily include committing
something. I won't be discussing the tooling for Zuul-like patch gating
at this stage, I'll just assume we can figure out how to implement that.
It would also be possible to build a patch gating system without
initially integrating it with our other tools, and if we decide that's
more important than the tracker workflow at this stage, we should switch
our conversation to what that would look like. Or perhaps we can work
on the two in parallel.
My own feeling is that in order to get maximum benefit out of patch
gating, we need more clarity and utility in our *tracker* workflow first,
but I do realize that how long it currently takes to commit a patch is
a significant pain point, and it might be better to address that first.
Goals
-----
In suggesting improvements to the existing workflow support, I'm
starting from the fundamental idea that anything we do in the issue
tracker and/or patch gating system should have either *operational*
implications or *important* informational implications. The former
should be more heavily weighted than the latter, in my opinion.
What I mean by this is that there should be no "busy work" setting
of attributes in our workflow: when you change the state of an issue,
it should mean that something new is *going* to happen to that issue.
Any purely informational attribute setting should be in support of that
new action, and wherever possible that information should *mean* something
to some part of our tooling, not *just* to the humans looking at the issue.
In general I want there to be as few clicks as practical involved in
updating an issue. However, I'm not addressing the actual UI design here
(although I have ideas); I assume we are going to iterate on it for a
bit until we have something we really like.
NB: This proposal includes a number of ideas from the "desired tracker
features" (http://wiki.python.org/moin/DesiredTrackerFeatures), but
by no means all of them. Some of the others are worth implementing,
but in this document I'm focusing on primary workflow, which I think
should happen first.
Roles
-----
Conceptual Roles
~~~~~~~~~~~~~~~~
In thinking about our workflow, I identify the following roles:
original reporter
commenter
triager
patch-producer
reviewer
core reviewer
committer
These are in the order that the role is involved in the issue (roughly;
variations are possible depending on the issue), and obviously a single
person can take on multiple roles during the lifetime of a patch.
I originally thought this list would have operational implications,
but it turned out to be only an aid in thinking about the problem.
I'm leaving it in here for exactly that purpose...it helps when thinking
about the states and state transitions.
I'd also like to add something that we currently only have informally,
but which has been requested as a feature more than once: at every stage,
I'd like there to be the possibility of there being a 'responsible party'.
This is sort of like 'assigned to', except that it could be anyone with
a tracker account, and the assignment would have a limited lifetime:
either until the issue's state changes, or until the issue has been
without update for some number of days (off the cuff I'd suggest 14,
but it might also vary by state depending on what states it actually
got used in).
The idea behind this is that we have eager contributors who either
wind up stepping on each other's toes, or rush to create and submit a
patch before someone else does, and as a consequence of rushing, do not
produce as good a patch as they are capable of, which actually slows
down the issue resolution. The ability to "take" a task and know it is
"yours" is an important part of the new contributor process, and having a
"responsible party" field would support that.
Tracker Roles
~~~~~~~~~~~~~
The tracker roles important to the workflow are:
User
Triager ('Developer')
Committer
Although the tracker calls the Triager role 'Developer', I'm going
to refer to it as Triager throughout this document, for clarity as to
its intent.
The tracker does not currently have a role equivalent to 'committer', but
we may not need to add one explicitly, since the account of a committer
is marked as such.
The important thing to understand about these roles in the context of
this document is that anything a user can do, a triager or committer
can do, and anything a triager can do, a committer can do.
Information Fields
------------------
An issue has an issue number and a title, and those wouldn't change.
It also has a state, which is the subject of the second half of this
document, and would likely be a new field in order to be able
to have both UIs available (this field is where the glue code
would be needed).
Beyond those fields, I suggest several changes to the issue metadata;
I will cover each one separately.
Versions
~~~~~~~~
Currently our versions field does double duty: we set it to the versions
we want to fix the bug in, and then deselect versions as we fix them.
However, I noticed when doing the "What's New" document that this
makes things really confusing if you *do* look at the ticket later.
You can't tell which versions got the fix without reading through the
entire issue. So I'd like to split this into two fields:
should be fixed in: [list of versions]
has been fixed in: [list of versions]
I'd also like patch set links to be displayed next to the versions for
'has been fixed'. When a commit references an issue, it should appear
next to the appropriate version, but it should *not* automatically
change the version state to fixed. That should require separate action,
since we sometimes apply patches that are relevant to the issue but
do not fix it. When an issue is transitioned to closed, there should
be an easy way to say "mark all versions with changesets as fixed".
It would probably be appropriate for that to be the default.
These fields have direct operational meaning: they indicate a task
to be performed or signal the completion of a task.
Type
~~~~
documentation
python bug
interpreter crash
security
enhancement request
I add 'documentation' here based on PyCon feedback from Jessica McKeller
and Selena Deckelmann. None of the existing types makes sense to a
beginner for a documentation bug, and the resulting confusion can lead
someone to abandon the effort to contribute. It also has operational
implications (see below).
I rename 'behavior' to 'python bug' for a similar reason, but I don't
have any usability data to back that feeling, so I'm not strongly
advocating that change.
I rename 'crash' to 'interpreter crash' in an attempt to reduce the
chances that someone will report a python traceback as a crash, something
that otherwise happens very regularly. I'm sure we'll never completely
eliminate those.
I drop 'compiler error', 'resource usage' and 'performance'. All of
these are bugs in the minds of the reporters, and classifying them
separately in the 'type' field does not, as far as I can see, lead to
any operational difference in the way the issue is treated.
What I mean by that: most bugs are either 'documentation' or 'python bug'
or 'enhancement request'. Differentiating these is important, as doc
bugs are handled very differently from python bugs (documentation fixes
do not get NEWS entries, for example), and bug vs enhancement determines
which versions we fix things in. 'compiler error', 'resource usage', and
'performance', on the other hand, are all handled with the same workflow
that applies to bugs. You might think that there is a difference for
resource usage and performance, in that we don't in general backport
those fixes. The key there, though, is "in general". The decision as to
which versions to apply the fix is made based on the magnitude of the bug,
and there really are only two cases: we mostly don't backport, as is the
case for enhancements, but we sometimes do backport, just like we would
a bug fix. The same applies in reverse to compile bugs: we mostly fix
those in all versions, but we don't always. So the only *operational*
effect of having these as distinct types would be to confuse things,
since we couldn't tell from the type whether or not this was something
that should be applied to all versions or only default. Instead we
should set (or, more likely, triage) them as either 'python bug' or
'enhancement', which have the correct operational implications.
There *is* an operational reason for having security and interpreter
crash as separate types. In both of these cases the versions we fix
it in is always the same (all active for crashers, all active + all
security-only for security bugs), and the issue priority should default
to either high or release blocker. In addition, security bugs should
be automatically reported to the PSRT, and arguably the report should
be be hidden from all but the PSRT and the original reporter.
Priority
~~~~~~~~
low
normal
high
deferred blocker
release blocker
The only change here is to eliminate 'critical'. I'm not wedded to
that, but if we have both 'critical' and 'high' priorities, 'high'
ends up getting treated as pretty much equivalent to either 'normal' or
'critical', depending on the person. I would argue that anything that
is severe enough to be marked 'critical' should in fact be a release
blocker, and anything that is not a release blocker is effectively only
'high' priority.
The priority is currently operational only in that one can sort issues by
priority. I propose that we make them much more operational, by posting
a count and/or list of the bugs with more than normal priority somewhere
public on a regular basis, such as python-dev, python-committers,
and/or #python-dev. (Well, definitely #python-dev.)
I'd like to see us strive keep those queues clear, so that promoting a
bug to high or release blocker means it *will* get acted on reasonably
promptly. (This raises the issue of what to do about bugs we currently
mark as "release blocker" as a *reminder* to the release manager. I don't
have a proposal for that at the current time, as release management is
out of scope for this document, but we'll need an answer if we are going
to implement this.)
Component
~~~~~~~~~
I propose that we completely change the nature of this item. I think we
should make it a text field that is filled in by autocomplete like the
nosy field can be, and that the value be any of the components listed
in the experts file. This would further respect how the experts have
listed themselves in that file by autonosying those that are willing
for that to happen. The field should still be a multilink (that is,
can contain more than one value).
This change would mean that it would be possible to search for issues
based on module, which is an often-requested feature.
Patch Status
~~~~~~~~~~~~
This is a new set of fields that records information about the patch:
unit test
fix
documentation changes
news entry
what's new entry
commit message
For each of these, the value could be 'needs work', 'complete', or
'not applicable'. For issues of type 'documentation', all lines
except 'documentation changes' and 'commit message' would be set to
NA by default, otherwise they will be set to 'needs work' initially,
except for "what's new", which will be set to NA for anything except
type enhancement.
Note that the inclusion of 'news entry' and 'commit message' assume two
things: that we retool the NEWS file so that NEWS entries can be added
automatically without conflicts, and that we change our patch workflow
to use mercurial's capability to accept 'hg export' patches and/or some
other sort of pull request. This part is one place we get into
non-roundup territory, so that may be a phase two addition.
Mercurial's 'evolve' feature, which we saw demoed at PyCon and which I
understand is currently available as a plugin, makes this capability much
more useful. When the non-core committer syncs the master repository
after their patch has been committed, the right thing happens to the
history in their repository to make their local commit disappear in
favor of the commit made to master. The contributor will also be able
to essentially *edit* the patch in their local repository based on the
review feedback, without ending up with a chain of commits in their
local repo that they have to deal with. Evolve should also facilitate
DVCS-based collaboration between core and non-core developers, as well as
between non-core developers. (I'm assuming all this works with exported
changesets, since in the demo they talked about starting with a patch
received via email, which is their normal workflow. But we should double
check this.)
These fields allow us to represent all the parts that a patch must have
to be complete, and they act as a checklist for making sure the parts
are all there. This is similar in intent to patchcheck, but since which
pieces are needed is ultimately determined by a human, it is more accurate
than that part of patchcheck and therefore more useful.
Stage
~~~~~
This goes away, subsumed into state.
Status
~~~~~~
This either holds the state information, or goes away in favor of a new
'state' field.
Keywords
~~~~~~~~
buildbot
easy
stuck
buildbot and easy are the only two existing, non-redundant tags that I can
see a way to make operational.
'easy' of course has to be set manually. The system for listing easy
issues should list the issues only when there is an action that someone
could take on the issue (clarify issue, write patch, review patch).
(NB: it might be possible to come up with a better name for this tag.)
For buildbot, those issues should be displayed in a report or dashboard,
and we should try to keep this queue empty. This becomes even more
important when we have a patch gating system online.
All of the other values are informational-only, and in some cases
redundant. If we want informational keywords, IMO we should do a full
user-accessible tagging system. That would be a separate proposal,
though.
Note: I claim the regression tags are redundant because I think
regression issues should be release blockers. (They are also ambiguous:
is '3.3regression' a bug in 3.3 that is a regression relative to 3.2,
or a 3.3.x bug relative to 3.3.x-1, or a 3.4 bug that is a regression
relative to 3.3? Better to just explain exactly what is going on in
the issue comments, IMO.)
The purpose of the 'stuck' tag is to label issues that we agree are
real issues that we don't want to close as "won't fix", but that we
can't for one reason or another currently move forward. Operationally,
stuck issues are either not displayed in work queues (see below) or are
displayed at the end of the queue.
I think the keywords should be settable by anyone, if they aren't already.
Issue States and State Transitions
----------------------------------
Here are the states that I think an issue can be in:
new
needs information
needs decision
needs decision from committer
needs patch
needs review
needs patch update
needs commit review
closed/fixed
closed/wont fix
closed/duplicate
closed/insufficient information
closed/postponed
closed/not a bug
closed/third party
I discuss each of these, and the possible state transitions, below.
All legal state transitions should be displayed in a particular
area in the UI, and each transition should be a single radio button.
Which possible transitions will be displayed will depend on the user's
roundup role, and occasionally other state in the issue.
The default transition is "no change".
Note that the names of these states are, unlike most of the other names
I've used so far, not the ones I necessarily expect us to use in the UI.
They are descriptive of the state from the workflow POV, not necessarily
the labels that should be used for the state *transitions*.
New
~~~
The issue has been created by the original reporter. The information
collected at this stage should be as simple as possible while gathering
the information we need. I propose, based on conversations at PyCon
with people who work with new users, that the only information we *need*
is the title and first comment. The rest would ideally be hidden/revealed
via javascript, with a per-user or browser-sticky setting that defaults to
'collapsed' for new users.
The 'advanced' section would allow the setting of type, component,
and nosy.
One could argue that it might be useful to collect 'found in version'
and perhaps other information such as os/version, but in my own use of
trackers I find that this is more often confusing than it is helpful,
and generally more detailed questions about the user's environment are
required in any case, making the up front collection attempt of dubious
utility.
A new issue can transition to:
needs information user
needs decision user
needs patch triage
needs decision from committer triage
closed triage, original reporter
needs review triage, if there is already a patch
needs commit review committer, if there is already a patch
That is, any user can indicate that more information is needed or
request that triage decide whether to accept or reject the issue,
but only triage can accept the issue as one that should be worked on
(needs patch), or close it, or request a committer to make that decision.
Operationally, new bugs get sent to the 'new bugs' list, and appear on the
weekly report in the new bugs section. We also have 'new bugs without
response' in the weekly report.
The list of bugs in this state constitute a "work queue", and should be
available as a standard search query and/or as a dashboard feature.
Triagers are primarily responsible for draining this queue, but anyone
can help.
needs information
~~~~~~~~~~~~~~~~~
We are waiting for more information from the original reporter, or
from anyone else who manages to reproduce the problem. If there is no
additional activity for some period of time (one month?) the issue is
automatically moved to "closed/insufficient information". A report
of autoclosed issues will be sent to appropriate places (python-dev?
new-bugs-announce? Anyone with triage privileges? Accessible from the
dashboard UI?).
The possible transitions are:
needs decision user
needs patch triage
needs decision from committer triage
closed triage, original reporter, autoclose
needs review triage, if there is already a patch
needs commit review committer, if there is already a patch
Bugs in this state are *not* a work queue.
needs decision
~~~~~~~~~~~~~~
A user has expressed their opinion about what should be done with the
bug, and confirmation is needed from triage. Note that an experienced
user can submit a bug and immediately move it to 'needs decision'.
There is nothing wrong with that, but it isn't necessary, since triage
will review bugs in new status just as soon (or sooner) than they will
review bugs in 'needs decision' status.
The possible transitions are:
needs information user
needs patch triage
needs decision from committer triage
closed triage, original reporter
needs review triage, if there is already a patch
needs commit review committer, if there is already a patch
This is a work queue and it is triage's responsibility to drain it.
We should strive to keep this queue empty. Issues where we can't
make a decision should marked stuck.
needs patch
~~~~~~~~~~~
The issue has been accepted as something we would like to fix. Now
a patch implementing the change is needed.
The possible transitions are:
needs review user
needs information user
needs decision user
needs decision from committer triage
needs commit review triage
closed triage, original reporter
This is a work queue, and it is something that everyone can help with.
It is the responsibility of the *community* to drain this queue, and
triager's or committers who write patches are doing so as part of the
general community, not in their specific empowered roles.
This is the primary place where someone might want to set themselves
as 'responsible party'.
needs review
~~~~~~~~~~~~
There is a patch, and it is ready to be reviewed.
The possible transitions are:
needs information user
needs decision user
needs patch update user
needs commit review triage
needs decision from committer triage
closed triage, original reporter
An issue cannot transition to commit review if any of the patch status
items is set to 'needs work'. Setting these can be done by any user.
It is always possible to upload a patch, regardless of state. It is
always possible to adjust the patch checkboxes regardless of state.
Anyone can do these things. So, a patch can get all the way to being
*ready* for commit review without getting beyond 'needs decision' state
(that is, without a triage person taking action), but it cannot get into
a commit review state without triage getting involved. The theory behind
this is that patch work is not *blocked* by triage/committer non-action,
but by the same token a patch can't get into the commit review queue
without one of those two actively deciding it should be.
Like patch generation, this queue is also a community work queue.
needs patch update
~~~~~~~~~~~~~~~~~~
The patch isn't perfect yet, but has had at least one review.
It might be nice for the reviewer to be able to easily restore the
'responsible party' to point to the patch submitter when moving the
issue to this state, or perhaps even for that to happen automatically.
The possible transitions are:
needs information user
needs decision user
needs review user
needs commit review triage
needs decision from committer triage
closed triage, original reporter
This is a community work queue. It is distinct from "needs patch" so
that contributors who want to find a new problem to work on can look at
the 'needs patch' queue.
needs decision from committer
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A question has come up that triage doesn't feel qualified to make the
call on. I envision this mostly as a training mechanism for triage:
the committer who responds should explain *why* they make the decision
they do. A triager who continues to reach for this button instead of
starting to make the calls themselves isn't a good candidate for further
responsibility.
Note that if committers are already nosy on an issue, there is normally
little reason to use this state, but it does move the issue into a queue
the committers are directly responsible for, so it can occasionally
be appropriate.
There is another use for this state: asking for a decision from the
maintainer of the module when there is an active maintainer.
The possible transitions are:
needs information user
needs review user
needs patch update user, if there is a patch
needs patch triage
needs commit review triage
closed triage, original reporter
This is a committer work queue. Note that the issue can be transitioned
out of 'needs decision from committer' by anyone, so that if someone
with more experience comes along it doesn't have to stay in the committer
work queue if that isn't in fact necessary.
needs commit review
~~~~~~~~~~~~~~~~~~~
Triage agrees that the patch is ready to be committed. This issue now
appears in a search/dashboard display for committers. Ideally this
queue should be kept close to empty, but in reality it probably won't
be until we've gotten the bug list reduced and more committers online.
This state represents the transition to the patch gating system. Until an
automated system is on-line, it would be the current manual system, and
patches could easily sit in this state for a while. Once we've figured
out the automated patch gating system, either the issue transitions
to that system until it is committed, at which point the state in the
tracker transitions to closed/fixed, or we will be adding additional
states and controls to roundup to implement control flow between the
tracker and the gating system.
The possible transitions are:
needs information user
needs decision user
needs patch update user
needs decision from committer triage
closed triage, original reporter
If any of the patch status checkboxes are changed to 'needs work', the
'needs patch update' next-state should automatically be checked.
This is a committer work queue.
Closed
~~~~~~
An issue can make a transition to any of the closed states from any other
state (even fixed...there are times an issue is 'fixed' by another issue,
and so an issue can be closed as fixed even without a patch having been
committed). You can also view the state as "closed-with-reason", with
the reason being the 'resolution' as currently managed by the tracker.
However, I think the UI for this should indicate both the close and
the reason with the same "gesture", instead of having to manipulate two
different dropdowns the way we do now. (And it's currently two only if
you ignore stage.)
Triage or higher role is required to close an issue, except that the
original reporter can also close an issue (as they can now).
A closed issue can transition to:
needs decision user
needs information triage
needs patch triage
needs decision from committer triage
needs review triage, if there is already a patch
needs commit review committer, if there is already a patch
I'm not sure if allowing any user to move a closed issue to 'needs
decision' is a good idea or not; currently closed issues can only be
opened by the original reporter or triage. But that has worked fine
for the most part, and the overhead of dealing with the people who have
abused it hasn't been much higher than those people generate anyway.
I'm sure those folks will be able to find some other part of this
design to abuse if we don't allow 'needs decision' here.
Dashboard
---------
The dashboard would be a new feature, a new landing page and linked from
the left hand menu bar. It would list the first N (adjustable) items in
each queue relevant to the user for which they are the responsible party,
followed by those on which the user is nosy, followed by those on which
they are not nosy.
A general user would see the 'new', 'needs patch', 'needs patch update',
and, 'needs review' queues.
A triager would see, above the preceding, the 'needs decision' queue.
A committer would see, above the preceding, the 'needs decision from
committer' and 'needs commit review' queues.
In all queues, issues would be sorted in priority order followed by most
recent activity, with color coding for the priority. 'stuck' issues
would appear last, and there should be something to visually differentiate
issues on which the user is responsible, just nosy, or not nosy.
As with everything else, this is of course subject to discussion.
It would also be lovely if the user could configure their dashboard,
but that would probably fall into the category of advanced features we
come back to later.
Big Picture
-----------
My goal here is to facilitate the involvement of the wider community
in our workflow as much as possible. The structure above is designed
to allow the community to do as much work as possible, and the "trusted
individuals" to act as gatekeepers to insure quality. It is theoretically
possible for a patch to get all the way to 'needs commit review' without
any more higher level involvement than a triage person moving the issue
to 'patch needed'. Of course in reality much more involvement from core
will be needed, since we need to transfer knowledge to the community,
and especially the newcomers, about our standards and procedures,
Not to mention the code. And committers are going to *want* to do
general commentary and triage level activities on issues anyway. But,
for those committers who have less time, I'd like to think that this
system would allow them to focus their time on just the stuff only they
can do, and thus perhaps draw more contributions from some of our less
active or currently-inactive committers.
The key to this is that instead of issues getting lost, core can watch
certain key queues: 'needs decision', 'needs committer decision', and
'needs commit review'. When issues get to those stages, the people with
the experience to do something know that it is *their* responsibility to
do it: the issues are to a point where the general community can't help
without core input. If we can keep those queues to a manageable size,
I think we can increase the amount of energy coming our way.
The other important goal is that via these explicit state transitions,
especially the 'needs decision' and 'needs commit review' transitions,
we get a much clearer picture of who in the community *has* absorbed the
core standards and procedures and is ready to be "promoted" to the next
level of responsibility. And the more people we get moved up, the more we
can get done. (Note: making that information *easily* accessible requires
additional tooling, but it should be possible and should be done.)
Note also that I tried to engineer this so that the structure does not
handicap us or add bureaucracy: even though the basic structure is that
you get triage signoff first and then committer signoff, if the person
doing the triage is a committer, they can just move the issue to wherever
it needs to be. So the only time two levels of signoff will be *required*
is when the committers are too busy to drain the queues. And work on
an issue should never be *blocked* by the system, the purpose of the
queues is to draw attention to issues that are "ready" for decision.
Thus the two levels help us manage the load when we need the help,
and don't get in our way otherwise.
That's my intent, anyway. You tell me if you think I'm headed in the
right direction.
More information about the core-workflow
mailing list