<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi<br>
<br>
<div class="moz-cite-prefix">On 09/01/2015 06:49 AM, Tim Sutton
wrote:<br>
</div>
<blockquote cite="mid:E0DC690F-1701-49BC-8CCE-BFBC461BF3F6@qgis.org"
type="cite">Hi<br class="">
<br class="">
<blockquote type="cite" class=""> I'm not sure this<br class="">
is really needed. There's a lot of PRs, but most of them are
either<br class="">
work in progress or waiting on changes from the original
submitter<br class="">
after developer feedback. Matthias already does a good job
keeping the<br class="">
list maintained and pinging various people for attention (so
that said<br class="">
- maybe he'd like funding to keep doing this!).<br class="">
</blockquote>
<div class=""><br class="">
</div>
<div class="">Hmm good feedback - maybe we should chat to Matthias
and see what is needed from his point of view…thanks!</div>
</blockquote>
<br>
I think a formalized process would be good to handle PR's but
unfortunately I don't have the perfect approach ready. Maybe
somebody can provide a good concept from other projects.<br>
<br>
Some general thoughts about it:<br>
<br>
Pull requests are a mix of ready-to-be-merged (or almost) and
prototype pull requests.<br>
The first ones are (from the authors perspective) ok to merge. The
second type is more asking for general feedback. Type 1 PR's are
often opened by external parties (possible future coders), type 2
often by core committers (used as show-room for your local R&D).
Therefore IMO the first ones deserve more attention. However, the
signal to noise ratio in the PR queue makes it hard to separate the
two, what scares reviewers off. And then there is a third kind of
old pull requests which are hard to merge because the code has
drifted away too far.<br>
<br>
I once introduced a label "prototype" so core committers could tag
their PR's as such and when looking through the queue one could
filter them. The tag has been removed. The use of the tag has not
been well-communicated at that time, this may have been the reason.
I think it would be worth thinking about if and how tags can be used
to improve the process.<br>
<br>
Closing a pull request (instead of merging) involves disapointing
somebody and dismissing possible improvements. I assume that
sometimes reviewers are scared to dismiss pull requests. If they do
not get merged anyway, they end up as zombies in the PR queue and
add more noise to the signal. I think a checklist of "what does a PR
need" would be good to point to and say "hey, sorry, but see, that's
why it's not possible to merge it".<br>
<br>
Some time ago somebody (I think Marco H.) used to assign reviewers
to pull requests. I think that this helped to motivate people to
perform review tasks. However, if tasks sticked to somebody with no
time or unwilling to review, these PR's would get forgotten. I think
by repeatedly pinging the responsible person or in case of
unavailability reassigning to somebody else would be good.<br>
<br>
Bottomline:<br>
It would be good to have a prompt reaction for type 1 pull requests.
It motivates people and early merging gives time to test before the
release (or even better before the freeze). Therefore it would be
great to keep the review queue of type 1 pull requests short. This
can be done by 1. quickly merging easy fixes, 2. merging after
asking the author for comments, explanations, improvements or 3.
closing / dismissing the PR.<br>
<br>
What I think is important is to find a good way to assign the
responsible dev for review, identify common causes which hold
reviewers back from proceeding and developing tools and measures
which help to counteract (like checklists and questions which may be
asked like "can you provide a test", "can you explain a
use-case"...).<br>
<br>
Sorry for the long text but I hope this helps to move forward,<br>
<br>
Regards,<br>
Matthias<br>
<br>
<blockquote cite="mid:E0DC690F-1701-49BC-8CCE-BFBC461BF3F6@qgis.org"
type="cite">
<div class="">Regards</div>
<div class=""><br class="">
</div>
<div class="">Tim</div>
<br class="">
<blockquote type="cite" class=""><br class="">
Nyall<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<blockquote type="cite" class=""><br class="">
<br class="">
_______________________________________________<br class="">
Qgis-psc mailing list<br class="">
<a moz-do-not-send="true"
href="mailto:Qgis-psc@lists.osgeo.org" class="">Qgis-psc@lists.osgeo.org</a><br
class="">
<a class="moz-txt-link-freetext" href="http://lists.osgeo.org/mailman/listinfo/qgis-psc">http://lists.osgeo.org/mailman/listinfo/qgis-psc</a><br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
Tim Sutton<br class="">
QGIS Project Steering Committee Member<br class="">
<a class="moz-txt-link-abbreviated" href="mailto:tim@qgis.org">tim@qgis.org</a><br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
Qgis-psc mailing list<br class="">
<a class="moz-txt-link-abbreviated" href="mailto:Qgis-psc@lists.osgeo.org">Qgis-psc@lists.osgeo.org</a><br class="">
<a class="moz-txt-link-freetext" href="http://lists.osgeo.org/mailman/listinfo/qgis-psc">http://lists.osgeo.org/mailman/listinfo/qgis-psc</a><br class="">
</blockquote>
_______________________________________________<br class="">
Qgis-psc mailing list<br class="">
<a moz-do-not-send="true" href="mailto:Qgis-psc@lists.osgeo.org"
class="">Qgis-psc@lists.osgeo.org</a><br class="">
<a class="moz-txt-link-freetext" href="http://lists.osgeo.org/mailman/listinfo/qgis-psc">http://lists.osgeo.org/mailman/listinfo/qgis-psc</a><br class="">
</blockquote>
<br class="">
<div class=""><span><img moz-do-not-send="true" apple-inline="yes"
id="F2090EE5-8F10-4F25-9D30-4E43FCE66688" apple-width="yes"
apple-height="yes"
src="cid:DDEF9B12-67C3-4498-BD7D-EC3563CC35A4" class=""
height="60" width="60"></span><br class="">
<br class="">
<br class="">
Tim Sutton<br class="">
QGIS Project Steering Committee Member<br class="">
<a moz-do-not-send="true" href="mailto:tim@qgis.org" class="">tim@qgis.org</a><br
class="">
<br class="">
<br class="">
<br class="">
</div>
<br class="">
</blockquote>
<br>
</body>
</html>