|
Please find attached my notes from the process BoF and discussion at the developer meeting. Discussion and comments are welcome. Lars == Code Reviews ==
The current workflow:
We tend to have race conditions when it comes to
reviews: the basic workflow is that reviewers tend to move patch reviews into a
queue (typically a mail foloder) and then work through them. Some reviewers are
faster/slower than others. For example: Jan and Ian are faster and tend to do
most of the reviews. This then leads others – e.g. Stefano and George – to
leave reviews to Jan and Ian. Discussion related to that topic: - Stefano: if I started a review and then Ian/Jan
respond to a review, I feel as if I wasted my time
- Jan: points out that reviews by multiple people are not a waste of time
- Stefano: this is more an issue of perception – often, I think I could have
done something else from my list and take this into account the next time (aka
do something else instead of a review). Do others have similar issues?
- George: if some reviewers are faster, then I often ignore a review –
aka the other series which has not yet been reviewed is the issue - Lars: maybe we need to try to actively coordinate a
bit more, given the increasing review load - Stefano, Jan: If we did this, the trade off is that
we are assigning work. Which Jan feels uncomfortable with. Revisiting the core issue:
It appears as if we are not using our review bandwidth as
efficiently as we could Lars: I just checked and we do not have any documentation
related to reviews.
- Wiki is not saying anything (ACK BY – REVIEW BY) - Review BY does not
need to come from maintainer - Jan: the process
developed over time – also see http://markmail.org/message/ngbgi6z33hbxfx7z
- Jan: Maybe requiring people to do a number of reviews to get their number of things committed? - Stefano: Imperially it is feeling a lot slower than the 4
days longer review time that Lars identified from commit data - Ian: queue folder also gets bigger Is there a tool that helps with keeping track who is
reviewing what? No, it appears there is not. E.g. Gerrit doesn't do it
either. Patchwork can’t tell whether a new version of a series has been posted. A related problem is that a thorough review (or someone new
entering the review process) later in the series hurts us
- Reviewers who come in after 3-4 stages, often cause
disruption if changes are cosmetic and no real bugs are found - Cosmetic changes should me made in early versions of patch
series - Jan: as a maintainer/committer one should push back on
cosmetic changes when found later in a series - Stefano: Partial reviews are hurting us more than other
reviews. In these cases, we review the first bit of a series. Then we get
interrupted by another activity – e.g. for a week – you don’t look at the code.
Then submitter re-sends and the entire series may have changed, causing extra
work for both the submitter and the reviewer. Proposal for Best Practice: Write a guideline
document to try to be more efficient and help newcomers contribute or review
patch series: - Encourage contributors to never be shy of doing reviews - Do architectural reviews as early as possible. At v6
only review for bug fixes - Try to review the full series as early as possible,
reduce partial reviews. - Add reviewed-by of people that did internal reviews.
The background here is that many organizations do internal reviews before the
code hits xen-devel. Konrad: REVIEWED BY INTERNALLY is less valuable –
Linuses viewpoint is that internal reviews should be ignored. We then had a
brief discussion about it, and how much we value an internal review depends on
how well the internal reviewers are known (the same as for external reviews) - Tell on IRC when you intend to review a big patch
series. - Try to CC the author of the code, not necessarily
the maintainer. - Add a tag on the subject to explain what the patch
is about (x86, arm, libxl). - Maintainers should encourage people to appropriately
tag their emails. - We encourage readers to add filters to the list to
be able to follow what they want. - If someone is willing to review, encourage them to state
that I am willing to review x amount of things a week - a team leader/maintainer
could then suggest a patch to review. Fear that this mechanism creates extra mailing
list traffic. - The document should define clearly what ACKY BY REVIEW
means and when to do reviews - A related idea is to propose some kind of mentoring
structure, for people who like to mentor and those who are willing to look for
mentorship == Mailing List Traffic == Problems: Mailing List traffic = making hard for newcomers
to get started
- Xen + QEMU – lots of email and distraction - New people don’t know what’s important and what isn’t and
where to start - Subject doesn’t always match the area – so filtering does
not always work Lars: could we use some auto-tagging based on code paths in
the email body (somewhere on the mail backend)
The concern that was raised was that it would make subject
lines too long. Maybe we can start to encourage people to use better tags.
We had a brief discussion on whether we should follow the
Linux model, where there is a hierarchy of lists.
Stefano: Xen on ARM – filtering does actually work, because
maintainers enforce CC. Linux uses
different lists for ARM, kernel, drivers, …
If we did this, we would end up with 3 or 4 lists if we did
it (could come up with some buckets: tools, core, x86, arm) … the danger is
that we would end up with false positives.
Also, there seem to be some wrong tagging with some mail
clients when mail is sent to several lists
ACTION: Lars to experiment and see what actually happens
Lars: maybe it is too early to try and resolve this. We
could look into how this works with the embedded and automotive list and see
how well this works further down the line == Developer Events == From a sponsorship viewpoint having two separate events is
less than ideal. It is cheaper and easier to find sponsors if we have a
combined event. My proposal would be to - Join Dev and User event into a 3 day event. The goal would
be to run the developer part as we always have, but add an extra day for users.
The 3rd day can then be used for developer meetings, board meetings
and working group meetings - We would leave the Hackathons as they are, but George suggested
that maybe we need a better name as in fact the Hackathons are npot Hackathons (suggestion:
Developer Meeting)
| 
