mumi CLI client features for review checklists

  • Open
  • quality assurance status badge
Details
3 participants
  • Arun Isaac
  • Ludovic Courtès
  • Suhail Singh
Owner
unassigned
Submitted by
Arun Isaac
Severity
normal
A
A
Arun Isaac wrote on 18 Jun 00:42 +0200
(address . bug-mumi@gnu.org)
87r0cv6vj0.fsf@systemreboot.net
The mumi CLI must provide features to help go through a review
checklist. A system that allows even those without commit access to
contribute meaningfully to the review process could be a big win. We can
use this issue to brainstorm ideas. One idea could be as follows.

# Idea 1

Projects provide a review checklist in their .mumi/config. For example,
something like

((review-checklist . (((name . good-commit-message)
(description . "Are the commit messages written well?")
(tag . review-good-commit-message))
((name . good-synopsis-description)
(description . "Are the synopsis and description written well?")
(tag . review-good-synopsis-description))
((name . tests-run)
(description . "Are the package tests being run (if available)?")
(tag . review-tests-run))
[…]))
[…])

When a reviewer checks one of these items (say the good-commit-message),
they run something like
$ mumi review --tick good-commit-message
and that sets the review-good-commit-message tag on the issue.

We could also have a status command like
$ mumi review --status
that lists the complete checklist with a tick mark by items that have
been checked.

This system is really a convenience wrapper around tagging. So, it can
be searched with something like
$ mumi search tag:review-good-commit-message
One might however argue that such searching is not very useful.

One possible downside is that this ties each project (guix, mumi, etc.)
to a single checklist. For example, what about guix patches that are not
for packages? Perhaps it is an idea to allow multiple checklists per
project.

Another downside is that this does not provide for multiple reviewers to
review and verify each other's findings. In other words, there is no way
for two reviewers to register that they both verified something
independently.

# Idea 2

A second much simpler idea is to implement templates for `mumi
compose'. Projects provide templates under .mumi/templates. For example,
$ cat .mumi/templates/review
[ ] Are the commit messages written well?
[ ] Are the synopsis and description written well?
[ ] Are the package tests being run (if available)?

Then, when reviewers review something, they compose an email like so
$ mumi compose --template review
that composes an email with this template. The reviewer puts an 'x' by
items they have checked.

The downside of this method is that this is just unstructured
text. There is no way for mumi to understand what parts of the review
checklist have been completed and thus generate useful reports,
filtering, etc.

Thank you for listening to my brain dump! Suggestions welcome.

Cheers! :-)
S
S
Suhail Singh wrote on 18 Jun 06:16 +0200
(name . Arun Isaac)(address . arunisaac@systemreboot.net)
87o77y3mx8.fsf@gmail.com
Arun Isaac <arunisaac@systemreboot.net> writes:

Toggle quote (4 lines)
> # Idea 1
>
> Projects provide a review checklist in their .mumi/config.

I think idea #1 can add considerable value. I'll note some opinionated
observations below.

Toggle quote (14 lines)
> For example, something like
>
> ((review-checklist . (((name . good-commit-message)
> (description . "Are the commit messages written well?")
> (tag . review-good-commit-message))
> ((name . good-synopsis-description)
> (description . "Are the synopsis and description written well?")
> (tag . review-good-synopsis-description))
> ((name . tests-run)
> (description . "Are the package tests being run (if available)?")
> (tag . review-tests-run))
> […]))
> […])

I think the fact that the names are distinct from the actions (the tag
that is to be added) is quite important. I.e., the "review-checklist"
should provide a convenience around "well-defined state transitions"
where the state is being recorded by usertags, but could also be
extended to include issue status (open, close, owner etc).

For instance as noted in

- When setting yourself as the reviewer, ensure that no-one else has set
themselves as the owner.
- Assuming above is true, when adding under-review tag, also add
yourself as the owner.

On a related note, perhaps "review-workflow" might be a better name than
"review-checklist"?

Toggle quote (5 lines)
> When a reviewer checks one of these items (say the good-commit-message),
> they run something like
> $ mumi review --tick good-commit-message
> and that sets the review-good-commit-message tag on the issue.

It may be important that when a tag is set, that others are unset. It
would help if this were possible to express via the review-checklist.
For instance as noted in

There are three possible transitions after a review is completed:
- Addition of reviewed-looks-good, removal of under-review .
- Addition of escalated-review-request, removal of under-review .
- Addition of waiting-on-contributor, removal of under-review .

The user-interface (as well as the configuration) should focus on those
transitions. Some transitions may indeed be simple and may only result
in the addition of a single tag, whereas others may be more complex.
Additionally, given the current state, only some transitions may be
"sensible" and permissible, i.e., it would help to be able to express
conditional/guarded transitions.

Toggle quote (3 lines)
> One possible downside is that this ties each project (guix, mumi,
> etc.) to a single checklist.

I think that that's actually a benefit. It can aid review of the
transition rules for them to be in one place.

Toggle quote (3 lines)
> For example, what about guix patches that are not for packages?
> Perhaps it is an idea to allow multiple checklists per project.

One way to address this would be to have the ability to have multiple
conditional "checklists" (or workflows) that are mutually exclusive and
exhaustive. I.e., support for nested "checklists" in addition to
guarding would be sufficient, but may not be necessary.

Toggle quote (5 lines)
> Another downside is that this does not provide for multiple reviewers to
> review and verify each other's findings. In other words, there is no way
> for two reviewers to register that they both verified something
> independently.

Could that be addressed by ensuring that the control messages are also
cc'd to the specific issue?

--
Suhail
L
L
Ludovic Courtès wrote on 27 Jun 14:21 +0200
(name . Arun Isaac)(address . arunisaac@systemreboot.net)
87v81u7ezg.fsf@gnu.org
Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> skribis:

Toggle quote (17 lines)
> # Idea 1
>
> Projects provide a review checklist in their .mumi/config. For example,
> something like
>
> ((review-checklist . (((name . good-commit-message)
> (description . "Are the commit messages written well?")
> (tag . review-good-commit-message))
> ((name . good-synopsis-description)
> (description . "Are the synopsis and description written well?")
> (tag . review-good-synopsis-description))
> ((name . tests-run)
> (description . "Are the package tests being run (if available)?")
> (tag . review-tests-run))
> […]))
> […])

[...]

Toggle quote (9 lines)
> # Idea 2
>
> A second much simpler idea is to implement templates for `mumi
> compose'. Projects provide templates under .mumi/templates. For example,
> $ cat .mumi/templates/review
> [ ] Are the commit messages written well?
> [ ] Are the synopsis and description written well?
> [ ] Are the package tests being run (if available)?

I prefer #2: it’s familiar to anyone used to Git{Hub,Lab}, it’s simpler,
good enough, and doesn’t go too far to provide a technical solution to a
social problem.

Alongside, a ‘mumi approve’ command that would set the
‘reviewed-looks-good’ tag would be a great step forward.

Thank you!

Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 71622@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71622
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch