I think this is a great model for development.  When it comes to submitting large new features or significant invasive changes, the onus must be on the original developer to make inspection and verification of incoming changes as easy as possible for reviewers.  In my opinion, this means pull requests of this nature should be accompanied by full explanation of what the code is doing, how it works, and how a reviewer should know whether it's working or not, including testing scripts.  Otherwise, it's just asking busy people to do more work.<br>
<br>For small bug fixes, I think it's enough to submit the pull request and have a few extra pairs of eyes look it over.  For bigger things, I think this is something to keep in mind.<br><br>Britton<br><br><div class="gmail_quote">
On Thu, Oct 6, 2011 at 4:12 PM, Sam Skillman <span dir="ltr"><<a href="mailto:samskillman@gmail.com">samskillman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi all,<div><br></div><div>I just wanted to relate the experience I just had with Matt's pull request on the isocontour flux calculations.  If you already regard pull requests as being awesome, you can probably stop reading.</div>


<div><br></div><div>The situation:</div><div>Matt had a fairly large set of changes that added new functionality to yt, and wanted another pair of eyes before pulling it into the main trunk.</div><div><br></div><div>The tool: bitbucket pull requests</div>


<div>Matt developed the changes in his fork of the main yt branch, then requested that they be pulled into the main repo (even though he had the ability to push directly).</div><div><br></div><div>The benefit:</div><div>

This allowed me to pull down his changes, test them, iterate back and forth with him, and make comments that now forever live in the "Accepted pull requests" part of the repo so that anyone can see why that was changed.  When it was set to go, I just clicked "Accept" and the changes were all merged in without incident.</div>


<div><br></div><div>Anyways, very effortless way to handle changes that are more than just one liners and need a collaborative effort before going into the main branch.</div><div><br></div><font color="#888888"><div>Sam</div>
<div><br></div><div>

<br></div>
</font><br>_______________________________________________<br>
Yt-dev mailing list<br>
<a href="mailto:Yt-dev@lists.spacepope.org">Yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br>