Good morning, I'm really pleased
to be speaking at Guix Days.
My talk today is about
improving our patch review and
my hopes for what it could be and
how it could be a lot better.
Let's get right in.
So first, a little about
me.
My name is Arun. I've been a
Guix contributor since 2016
and a committor
since 2017.
Guix is what got me really interested
in Guile. I've
been interested in the state of the issue
tracker for a long, long time.
Since
the sad days of the Debbugs
web interface before Mumi
even existed. I am
the author of guile-email and guile-[sapien???]
both of which are
essential to Mumi.
So the story is that
I was... So, before Mumi,
existed I was writing
a new web interface for
Debbugs, the Debbugs tracker,
and unbeknownst to me
Ricardo was doing the same thing.
And he
he had listed before
I could.
He listed his version before I could.
And
And I looked at his code
and it turned out that his
web interface
was a lot better than mine and
my email [password???] was better.
So, we had
a chat and we decided that the best thing to
do is combine forces.
So, I released
my email-[password???] as Guile
guile-email and he released his
issue tracker as Mumi.
And
that's the way it's been.
So
This talk
could not have been without
many, many other
influences. But most of
recently I want it highlight
a few talks and articles
that I read recently.
I want at least to
point you to the first
talk, "Governing Rust" in the
ninth RacketCon.
It's a
really good talk and
it's extremely
relevant to the kind of problems that we
are facing in scaling up
the Guix community.
I urge you to watch it if you
haven't already.
Guix is a victim
of its own success.
Users, contributers,
and contributions have been growing rapidly,
I suppose exponentially.
This is a really good thing for
the project, but we're not able to
keep up with the
work load. That's the sad truth.
Issues are
falling through the cracks
and reviewers are
silently
tuning out.
The firehose of information that
is the email
every day. That is way too much mail than
any single person can
keep track of and it's only going to get
worse.
But how bad it can be, just have a look at the
(GNU/:)Linux mailing list.
Nobody can keep that [perfect???].
So,
there is quite a lot of frustration about this
among the community.
what the contributors,
reviewers, and pretty much anyone...
It's not really easy to see this in public because
people are polite
and want to stay polite on
the mailing list. That's a good thing!
But in private conversations,
it's pretty clear
and evident.
So, in fact that is often THE topic of
conversation in discussing Guix.
So, let's look at the
problem from two points of
view; of two
stakeholders. First, the contributors point of view and
then the reviewer's point of view.
So, from the contributor's point of view
they want
the reviewers to be polite and helpful.
That's something Guix has done an
exemplary job of and
something I'm really, really impressed
about the community and the reason
I stay here. So,
we don't have to worry about that
in this talk at least.
And second is
that documentation should be available
to, you know, understand and
hack on the source.
I think they've done okay in this
regard. It's probably
lots of scope for improvement and the subject
of other talks in Guix Days.
But, okay, not bad.
Not too bad.
And then third:
the contributions should
be reviewed and accepted quickly. This is
the real pain point and the
subject of this talk. I think
speed is the most critical factor
in capturing and sustaining interest.
So,
when a new user or a new contributor
comes in with a patch,
you want to be able to get back to them really
fast.
A day, if that's possible.
And certainly a week is a good
time to shoot for. Anything
more than a few weeks and
going to the months and years is
terrible.
People are extremely likely
to lose interest and never come back.
So, if we give this
up some point, we're going to pay the price and
lose out on new
useful contributors and that's not good.
So what about the reviewer's
point of view? Of course,
it's easy to blame the regulars,
but they also have
way too much work and
can't really keep up.
So, from their point of view,
it should be
quick and easy to
review and merge
the patch.
Having lots of back and forth is not
desirable.
You want to, I really want to see
one very
well-written patch
that can be, you know,
just merged at the click of a button, so to speak.
And it should be easy,
even trivial, to test these
patches. Sometimes when your
patch is involving
large packages
it takes
a really, really long time to
build those packages and test them.
Right, so if you're spending 10 hours
building a package
and then you tweak a little bit and
a little
bit in the package definition and you want to try
building it again...then good luck with that!
That's a really long
edit, compile, and debug cycle.
And you can't even know ahead
of time how long that package is going to take
to finish building. So,
you probably be
switching buffers frequently
and glancing back at that
task, even when you are
probably trying to do something else.
This is, of course, terrible for
productivity and really going to
kill your productivity.
Right.
So, something needs to be
done about that. And then finally,
reviewers want to
receive notifications only about
some specific subsystem that
they are interested in.
Nobody can keep up with all the information
coming through the mailing lists
and there should be some way to
subscribe to
only patches of the
huge patch inflow.
To make it
quick, easy, and make it trivial. That's kind
of the idea.
So,
the first thing is to make it easy
for contributors to produce
high quality patches.
So, there are many slipping
points for the
contributors.
And if they
can be made simpler,
and if contributors could produce
higher quality patches at the
first attempt, they need to really
lighten the load an the reviewers. And then
we need to make
it easy for the reviewers to
do that, lighten
the work of reviewing. So, how exactly do we
do that? I have a few
mockups and I'll
try to explain with those,
with the aid of those.
So, let's look at...
I hope you can see the
issue tracker here. This is
what Mumi normally looks like.
We have patches listed. We have issues
listed here. And
that is...
this patch
tag, called a patch,
and that's pretty much the only
tag that you see here.
I believe we want something
more like this.
You see lots of colorful tags.
So, package-fix, package-update,
new package, and so on.
So, package-fix
package-update, and new-package are the three most
common operations on a
package. And then you have these
black tags which
tell you which subsystem that
issue is about.
So, gcc is in
commencement.scm.
Emacs packages are in
emacs-xyz.scm.
There's a python-crypto package
and so on.
And actual
bugs in the
core Guix [missionary???]
are shown using
these red tags.
And so on. You have a bug here and so on.
So,
the thing is, we have
two broad kinds of issues,
ones that deal with the core of Guix
[missionary???] and
others that deal with packages.
Package [???] issues
are never ending and
[there are] a huge
number of packages and a huge number of
issues.
Our issue tracker tends to be swamped
with those kind of issues.
It's hard to find, like,
bugs which are related to the
core. You know, dig through all the issues
find only those that are related to
the core of Guix's mission.
So, we want, like, tags could
really help us sort through this mess
quickly and
get people to the thing that they
are interested in.
And also I replaced
the absolute date
here with the relative
date. That's a pretty easy fix,
just a one liner patch.
I'll probably send it this week to Mumi.
Mumi already has most of the code
to do it.
So, here's the next page.
This is what an issue
normally looks like.
You have a title
followed by the conversation, actually
the email conversation, here.
What I've added a-new is
this section called "Summary and pointers".
So,
issue conversations tend to be
long and often tedious, involving
lots of details and following
the train of thought of multiple
participants.
Someone looking
at the issue should not have to dig
through the whole thing to understand what it's about.
So,
It would be helpful if
someone, one of the participants or someone else,
put in a summary
[of the problem], summarized the problem and
suggested a good way
to fix the problem. So,
this is a great idea
for more experienced folks
to do because
we want experienced folks to
be doing the kind of thing that
no one else can do.
So,
if they were to...
We don't really want
them to
code too much and
get in all the nitty-gritty of
all those packages out there.
Rather, if they could
write out these detailed instructions
to people,
instructions in every bug report
explaining what to do, how to fix the issue
and where to fix the issue.
Maybe with examples, clear examples.
Some really well written...
...a few paragraphs.
That would be of immense help to others.
New contributors could
learn from them and
and they might themselves become
more experienced
contributors quickly.
That's one addition.
Another [connotation???] here then
a patch here. And
then you have two little check boxes,
two little tick marks.
So, you want...
as soon as a patch comes in we want
lint tests to be
automatically run and
also the package,
in order to be rebuilt in the CI (continuous integration)
and then we want
little check marks like this which
come up and indicate that it passed
correctly. I believe
Guix
data service is supposed to do something like this
and I believe that
Chris Baines has been...
has a patch work instance
that indicates the
status of each patch.
I haven't looked at it very
closely, to be honest.
But,
I believe we want
that kind of information to be integrated into
Mumi itself. We don't want that to be
available on a separate,
you know, separate period, a completely
different interface and so on.
That's one... and
Then finally, review check lists.
So, as much as possible, the
the review process, the tests have to be automated.
Well, of course, everything cannot be automated.
So,
for those tasks that can not be automated,
we should have a check list.
So, review is largely about
looking at
lots of tiny things and
it's pretty easy to forget the kind of
all the points that you have
to check. So, it's nice
to have a check list. In fact, this is the
exact check list that I
use personally when I'm reviewing a
patch.
I have this in an Org mode file and
I just go... I run
through the check list one point
at a time.
It would be nice if
this kind of check list got standardized and
available to everybody so that we could
just...anybody could just,
you know, run through it and...
We would have a standardized process
for all
reviewers to implement.
And they can have some
confidence that they are doing the right thing.
And often we find the same
kind of mistakes in every,
in many different patches.
[indistiguishable???]
Really similar responses to people.
So, what
if you could just have some check boxes and,
you know, if the tests are not packaged
and check that.
Something wrong with the indentation? Check that.
And then just
"Report failing checklist items"
some kind of template mail goes to the
goes to the issue.
That would be pretty
convenient, right?
Yeah.
So, the...
One point I'd like to make about the
tooling is that...
So, as long time
hackers on this
project, we tend to accumulate
a bag of tricks, right?
So, for example, how do you
apply a patch from a mailbox
from the convenience of your mail client
directly to the Guix git repository?
So, for a long time I
didn't know that. I didn't know how to do that.
And then I hacked
up some [???],
integrating Notmuch and
magit and all that.
So,
but no one should have to,
like, hack up this kind of stuff
and reinvent the
wheel on this. We should
have a standardized
and regularized tooling that
anybody can
just start using without
having to reinvent.
So, that's idea of tooling.
Now let's go on
to the shell where I have some...
one more mock-up.
So,
you see this
package here? I have this package
called "borgmatic" here.
It has a couple of linter errors.
Let's see how
we normally report them.
So,
let's `./mockup guix lint`
This is normally how
`guix lint` reports
linter errors. So, there's
a filename, line number, column
number, package name, package version,
and then some textual description
of the error, right?
There are two linter errors for this
case. What I propose is
something more like this:
Something
much more detailed, colorful, and
most importantly,
it has
the in-context
source of the error. So,
The first error is about
missing double spaces.
So,
here you see in the context of the source code
exactly where the double space is missing.
This is, of course, far better than having to say
possible infraction at 86.
Here, I don't
believe anybody really counts
up to distinct position 86 and finds sort of
that the... missing
double spaces are, right?
That's crazy.
So, if you could...
Like, iron
this out and
exactly show where the error is would be far better.
And the second part
of this is this line in magenta.
That is the rationale for this rule.
So,
Humans really like to understand
why we have to follow all these rules.
It's frustrating to follow rules that we do not
know why we are following.
So, for a long time, until recently
I didn't know why we needed to
add two spaces,
you know, after a full stop,
after a period.
And it turns out
it's because we need the...
it is great for Emacs and [text commands???]
to work, right? So, it
will be
really good if this kind of rationale were
put there so that people can
understand. And...
Even though I did not know this
for a long time, even though I did not know
why we have this rule for a long time, I
I never wanted to ask about this because it sounded
like a stupid question which people should not have to ask,
right? And that's exactly what
new users are going to do. They're not going to ask you
these questions. They're just going to
maybe be bad with it or
they're probably going to be done [indistinguishable???] or something.
Alright, so we want this rationale there
definitely.
And the second
that I'm going to say just about
check face
not respecting tests.
So, in the
context source, you see exactly the
fix that is needed to
address this linter error.
That's nice.
So, it might seem that
that this tooling
definitely needed to create this kind of
tooling would be too much.
But
I think it'll [certain a lot of saving???]
saving of user
frustration and also of
reviewer time. So, if the
contributors have...are producing
high quality patches without
any errors... there's...that'll
already save a great deal of time for the
reviewers and is really good for the project.
So, back to the slides. So, quick
recap of the mock-up that I showed:
colorful tags, guidance
and mentoring from experienced folks,
more automated
review, and then
checklists for tasks
for review tasks that cannot be automated
and [then lint error messages???].
So, to the second part of the talk, I believe
that our review
model is broken.
So,
normally we have one contributor
who comes with a patch and
one reviewer who interacts with them
and,
you know,
there's a lot of back and forth that goes in and finally
they come down to a patch
that can be accepted. So, usually
at least more often than
not, there is just
a single reviewer per issue.
For whatever reason,
people don't like to barge in
on somebody else's issue, right?
It happens sometimes, I mean there's times...
It's not a bad
thing that, you know, commenting on
some....on a...
on an issue that somebody else is ready working on
is no a bad thing. I think it's pretty
good in giving a
second opinion to the whole process.
But more often than not we don't
do it.
This
puts a lot
work and responsiblity on that single
reviewer. And review is a pretty
exhausting process.
So, I think we should...
I suggest that we split up
the review process into multiple
phases. Let's see
how.
So,
Step 1 should be the
triaging process.
If we prescreen the bugs, the issues
that come in, classify
them and tag them with our
eleven tags, maybe
ask for more information from the
contributor, if necessary...
And that's it. Triaging
is done. So, hopefully
this can be done in less than
a day so that
the contributors
get some, you know,
positive feedback where
they can feel good that they got a, you know,
an immediate response here. And
a promise that their bug
will be addressed in some
time at least.
And we
ought to have a
a separate team
just for triaging. You don't
really
want them to be
getting involved in
the review process, unless they want to, of course.
So, that...
the responsiblity of the
triaging team is finished as soon as
the triaging process is complete.
They should not have to follow up that
single issue all the way down to the end, right?
So, triaging is
really easy, anybody can do it, even
really inexperienced
folks can do it pretty easily if...
system... basic...
[assigning tickets???] type of thing, right?
And then we want
the...if there's a bug
and...
we want the more experienced people to
come out and
write instructions on
how to address the
bug and, you know, maybe
which lines of code, which functions are relevant,
you know, that kind of thing. So,
as far as possible, we want to do
the kind of things that
nobody else can do. We don't want
them to
to be
doing all the grind work themselves. They should
also be teaching others and this is a great way.
This is the summary paragraph
that I read at the top of the
mock-up. Then maybe
the first round of review could come from a
non-committer? I don't know how
we could motivate them to do it.
But, if
[???] non-committers that
would be, that would take off significant
work from the reviewers...
from the committer reviewers.
And it might also be a good way to
for us to, you know, find potential new
committers...
people who we can,
you know, give them a more integral
part in the committing [???].
And apart
from this we also need to
broaden our idea of
what community means.
It's still pretty tempting to
think of the community as just a
you know,
core developers and users.
I think that's a pretty bad approach, I mean
More and more we realize this but
there are still some
tragic hangovers from
ages past, like the idea of...
it's so easy to
only credit
people commit in core
because all they have to do is run a git log and
you know, a little shell pipe
and you have a list of all contributors, right?
And this tends to go into the
news reports in the
[???]
We do it because it's an old habit
and it's
something so easy to do, you can
even prefix it with the number of commits they
did and it looks
really neat.
Probably follows the power law or something, 80-20 and
everything.
But it's kind of, you know...
it reflects an old
mindset that only quotas are
real contributors,
right? And that's not a good
thing. We should be also
looking at
people who manage
the community, so to speak.
Like people who answer support requests.
People who are doing triaging are probably not
going to be...
involved...at all showing up
in the git log. But they are contributors
and essential to the community nonetheless.
And then we need toolsmiths
who work on tooling
like Mumi.
Again, it's not going to show up in the Guix
git log but
the associated tooling is essential for Guix to
operate well.
Software communities are
just communities
of people,
So whenever you put a bunch of people
in a room, you're going to have trouble and
software is no different.
Software
is about the people, it's not about the code.
When you have the right people
and the right kind of
organization, code will come on its own.
So,
Mumi, as you know, is
our interface
to Debbugs.
And
hacking on Mumi is kind of the
point of all these mock-ups.
So, the Debbugs API is
terrible.
If you never had a chance to look at it,
you should be really happy. :)
It
does not expose...
It uses a SOAP...
an old XML
a remote
procedure call standard.
I could live with that, but
it does not
expose really important
attributes
on debugs, for example.
It won't even tell you the number
of emails that
have been received relevant to a certain issue.
Right? So, really basic
things it does not expose and
it's
almost any work done with it.
So, Mumi is a great
improvement and I'm happy it exists.
Mumi is
actually on an IP
allow-list to sync data from Debbugs
It has special permission from
the Debbugs admins and
it is on an IP allow list.
So, this makes it really hard
to, you know, run your own instance and...
of Mumi and hack on it.
So, I'm hoping...I'm thinking
that a Mumi API will
let us expose the
data that
Mumi is the custodian of, right?
And, recently I've
been hacking on a GraphQL API for
Mumi. And
also a GraphQL library
for Guile in general.
I hope though
this API will...
So, this API is already in Mumi,
even though I haven't properly published
[???] and documented the whole thing.
The hope is that
it'll motivate the creation of many
small, lightweight clients.
I say, "lightweight"
because
Mumi is already doing a great job of maintaining
the state of
all the issues.
The "state" meaning all the email
conversations, the
current state of the bug,
all of the persistent state, the state that's
in functional programming.
And...
More often than not, clients should only
download only the specific bits
of state that they want...
Use it and then, once they're done
with it, discard it...
because
if you're...we already have
way to many emails and if you wanted
to download all of them into your local machine, it's going
to be a few...
At least a Gig or few...
Gb (gigabytes).
So, [I believe the???]
Linux kernel mailing lists are
200Gb or something and that's
way too much state to hold on your
local machine.
So, we are also heading in that
direction, the direction of
the Linux mailing list and
downloading [our???] entire
state
kind of like...git does it....this really bad idea...
We want something more lightweight.
So, hence I thought GraphQL
was really nice and it was fun
to hack. So, that's kind of the
reason.
So, thank you!
Now you can have
(End of recordingabout)