[Ecosystem-engineering] Proposal: New review queue and review process

[Cory Johns]

Marco,

I just wanted to follow up and say that this proposal sounds great to
me.  The only addition I would make is clarifying what the logic is
when there are conflicting reviews; e.g., if you mark something as
TRIAGED (I don't suppose there's any possibility of getting a FIX
ACCEPTED status added?) but I mark it as INCOMPLETE, does it get
hidden / sorted down on the main review queue or not?

In addition to having it formally defined, I also think it should be
clearly and easily visible from the review queue itself; that is,
there should be an obvious "How Does This Work?" link that displays
all of this information for reviewers and reviewees alike to see.


Thanks for working on this.  I think this will be a definite
improvement to the review process.

- Cory

On Mon, Aug 18, 2014 at 1:59 PM, Marco Ceppi <marco.ceppi at canonical.com> wrote:
> Hi everyone, I'm going to jump right in with this email be briefly
> describing a short summary of the issues people have brought up around the
> review process and the review queue, then outline what my proposal to fix
> this is, outline the new process for feedback prior to implementation,
> finally listing actions left to implement this. The current review queue has
> been an indispensable tool for helping maintain the ecosystem as it is today
> and the code that the new review queue is built on shares a lot of the same
> logic/concept. However, since we've grown a lot in the last few years this
> is an attempt to help bullet proof the process as we move forward.
>
> # Issues
>
> One of the biggest feedback we've gotten from users lays along the lines of
> "where is my charm in the review process and what are next actions".
> Currently, the review queue doesn't illuminate any next actions for a user.
> As queue times increase this can leave a poor user experience. Another
> feedback point we have seen recently are reviews being lost in the process.
> This is arguably the primary reason behind a revamp of the review queue and
> review process to ensure a submitted item for review does not get lost in
> the process. These are two primary examples, while a more exhaustive list
> exists these items have the most impact for users contributing to the
> ecosystem.
>
> # Solution in Development
>
> For the past two weeks I've been working on a replacement review queue which
> uses a lot of the initial code from the current review queue. A few major
> points:
>
> * Ingests all bugs and merges regardless of state, owner, or series
> * Tracks reviews once ingested
> * Decoupled from charmworld
> * API access
> * Detailed statistic gathering
> * Track reviews per user
> * Track merges from LaunchPad and Github
> * Priority sorting of reviews
> * Track multiple queues of work at once
> * Modular code base for extensiblity
>
> This is what has been done for the first version of the review queue which I
> plan to release this week for review. However, as the review queue is
> currently tracking ALL items (and not items for which only a charmer can
> take action against) we will need to update the review process for charmers
> and charm-contributors (Jr. Charmers) alike. Those proposed changes are
> outlined below:
>
> # Proposed review policy changes
>
> ## New Charms
>
> With the new review queue, charmers being assigned to the bug is no longer a
> requirement for it to appear in the queue. Any bug against the charms
> distribution which isn't targeted at a source package (charm) will be
> ingested. Currently, "NEW" and "FIX COMMITTED" charms appear in the queue.
> Going forward, LP statuses will have more effect on the queue. "NEW" status
> will appear in a separate queue. These are charms that are just being
> submitted. The new queue is designed to get first contact to that user
> faster. "TRIAGED" should be what a reviewer (who isn't a charmer) moves the
> bug status to if it passes an initial review "FIX COMMITTED" will indicate
> that the charm author has received review items and is ready for another
> review, "INCOMPLETE" or "IN PROGRESS" indicates that someone has reviewed
> the charm and it needs work to complete. "FIX RELEASED" when the charm has
> been promulgated. All other statuses indicate an abandoned/closed review.
>
> ### Proposed workflow
>
> 1. User submits a new charm by opening a bug on Launchpad and linking a
> branch to that bug
> 2. It appears in the incoming review queue, any person (charmer,
> charm-contributor) provides initial review (Welcome, thanks for submitting,
> proof, etc) and outlines the process for the user by either linking to the
> documentation or use of boilerplate text. If the charm passes initial
> review, reviewer moves bug to TRIAGED, if it fails, moves bug to INCOMPLETE.
> Author will need to move bug to "FIX COMMITTED" once issues are resolved.
> 3. Once bug is in TRIAGED, a ~charmer will review the submission. If this
> passes review charmer promulgates and moves bug to "FIX RELEASED", otherwise
> places the bug in "INCOMPLETED".
> 4. If further action is needed, once author resolves, moves bug to "FIX
> COMMITTED" to have it re-reviewed.
>
> ## Updates to charms
>
> When reviewing merges to a charm, the merge proposal no longer needs to be
> assigned to charmers to show in the queue, any merge proposal against a
> promulgated charm branch will appear in review. Likewise, voting on a merge
> proposal will not directly remove it from the queue! So feel free to vote in
> the affirmative/negative often when reviewing merges. If a merge needs work
> you will need to move the merge status from "NEEDS REVIEW" to "WORK IN
> PROGRESS" which will remove it from the queue. The author then needs to move
> the review back to "NEEDS REVIEW" for it to re-enter the queue.
>
> # Actions
>
>  * Seek feedback and approval of updated process
>  * Release first version of review queue
>  * Clean up outstanding (no longer valid) items
>  * Update documentation to reflect new review process
>
> I'm interested in concerns, feedback, and suggestions as I continue to work
> on an improved charm experience.
>
> Thanks,
> Marco Ceppi
>
> --
> Mailing list: https://launchpad.net/~ecosystem-engineering
> Post to     : ecosystem-engineering at lists.launchpad.net
> Unsubscribe : https://launchpad.net/~ecosystem-engineering
> More help   : https://help.launchpad.net/ListHelp
>