<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">It seems to me that something probably ought to be done, because most of the analysis modules are so domain-specific that they often require expertise in that particular area in order to give a proper review beyond just code errors and/or style. This is often what happens, in PR hangouts I often hear “well, the code itself looks good, even though I have no idea what it does."<div class=""><br class=""></div><div class="">photon_simulator is an example of this (albeit an extreme one). I’ve contemplated spinning it off into a separate package, given that its size and complexity are both getting rather large, but given that we have more than a handful of users for it I feel that this would be too disruptive.</div><div class=""><br class=""></div><div class="">As an example of how to do “affiliated packages”, AstroPy has such a notion, but I always found the process by which a package achieves this status rather complicated. </div><div class=""><br class=""></div><div class=""><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__www.astropy.org_affiliated_&d=BQMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=hgcBC3x6dKFoTrmFmMYYbKNfiHZlGLKliIidd1LwmHI&m=kW1_fwfFTti2lMI7gO2kvyk104mlG15DxyGLw6CHK50&s=Qdog6bJvUPib9Xy060IygOf-6HX6_jL5SNOjMIcc-2g&e=" class="">http://www.astropy.org/affiliated/</a></div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 30, 2015, at 5:25 PM, Matthew Turk <<a href="mailto:matthewturk@gmail.com" class="">matthewturk@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi all,<br class=""><br class="">This is an interesting discussion ... and to be honest, I'm kind of<br class="">left wondering if maybe we should be encouraging more analysis modules<br class="">to be external packages. I came into this thinking that we would want<br class="">to make analysis modules behave more like external packages that get<br class="">bundled, but I'm not sure that's the case after all. Unfortunately,<br class="">we don't *have* a method for defining "affiliated" packages, and we<br class="">certainly don't have (or *want*) an update mechanism for external<br class="">packages that build on yt. But this becomes something of a<br class="">self-reinforcing loop ... Maybe we should come up with something like<br class="">that, so that projects that build on yt can be "advertised" somehow in<br class="">the base package, or something. I am not sure.<br class=""><br class="">Anyway, the responses to this thread make me think perhaps we<br class="">shouldn't lower the review barrier after all -- going from 3 to 2 is<br class="">perhaps just window dressing, and maybe we should encourage the<br class="">analysis modules to have higher bus factors.<br class=""><br class="">-Matt<br class=""><br class="">On Tue, Dec 29, 2015 at 5:09 PM, Nathan Goldbaum <<a href="mailto:nathan12343@gmail.com" class="">nathan12343@gmail.com</a>> wrote:<br class=""><blockquote type="cite" class="">I agree. If we can identify maintainers for analysis modules, I think only<br class="">one additional approval besides the maintainer is fine. If the maintainer<br class="">proposes the PR, they should try to get at least one person to look over it<br class="">in detail before it gets merged.<br class=""><br class="">On Tuesday, December 29, 2015, Britton Smith <<a href="mailto:brittonsmith@gmail.com" class="">brittonsmith@gmail.com</a>> wrote:<br class=""><blockquote type="cite" class=""><br class="">I agree that the number of approvals is less important than ensuring that<br class="">PRs come with appropriate tests and docs. If the PR is a change to an<br class="">existing analysis module, we should just make sure the relevant devs are<br class="">identified and that they sign off on it. If it's something brand new, I<br class="">think 2 or 3 people is probably still a good idea.<br class=""><br class="">On Tue, Dec 29, 2015 at 4:44 PM, Matthew Turk <<a href="mailto:matthewturk@gmail.com" class="">matthewturk@gmail.com</a>><br class="">wrote:<br class=""><blockquote type="cite" class=""><br class="">Ideally, with the barrier to acceptance reduced, the reviews that come<br class="">in may be deeper and more thoughtful ... and less along the lines of<br class="">"Can somebody just hit the approve button?"<br class=""><br class="">On Tue, Dec 29, 2015 at 4:41 PM, John ZuHone <<a href="mailto:jzuhone@gmail.com" class="">jzuhone@gmail.com</a>> wrote:<br class=""><blockquote type="cite" class="">+1 for lowering the barrier. At a minimum, tests should pass, and new<br class="">tests<br class="">should be encouraged if possible. Docs as well.<br class=""><br class="">Not too worried about 1 or 2 approvals. We could try 2 at first to see<br class="">if<br class="">this helps things out.<br class=""><br class="">On Dec 29, 2015, at 5:39 PM, Cameron Hummels <<a href="mailto:chummels@gmail.com" class="">chummels@gmail.com</a>><br class="">wrote:<br class=""><br class="">As long as the code being changed is local to an analysis module and<br class="">not<br class="">used by other parts of the main yt codebase, yes, I'm all for dropping<br class="">the 3<br class="">reviewer requirement to 2. 1 might be pushing it though.<br class=""><br class="">On Tue, Dec 29, 2015 at 2:38 PM, Matthew Turk <<a href="mailto:matthewturk@gmail.com" class="">matthewturk@gmail.com</a>><br class="">wrote:<br class=""><blockquote type="cite" class=""><br class="">That's precisely what I had in mind.<br class=""><br class="">On Tue, Dec 29, 2015 at 4:36 PM, Nathan Goldbaum<br class=""><<a href="mailto:nathan12343@gmail.com" class="">nathan12343@gmail.com</a>><br class="">wrote:<br class=""><blockquote type="cite" class="">On Tue, Dec 29, 2015 at 4:29 PM, Matthew Turk<br class=""><<a href="mailto:matthewturk@gmail.com" class="">matthewturk@gmail.com</a>><br class="">wrote:<br class=""><blockquote type="cite" class=""><br class="">[+-][01] on reducing review overhead for analysis modules?<br class=""></blockquote><br class=""><br class="">Does this just mean reducing the number of PR reviewers before we<br class="">merge<br class="">pull<br class="">requests? I'd be ok with that, but just want to clarify what you<br class="">have in<br class="">mind.<br class=""><br class="">_______________________________________________<br class="">yt-dev mailing list<br class=""><a href="mailto:yt-dev@lists.spacepope.org" class="">yt-dev@lists.spacepope.org</a><br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""><br class=""></blockquote>_______________________________________________<br class="">yt-dev mailing list<br class=""><a href="mailto:yt-dev@lists.spacepope.org" class="">yt-dev@lists.spacepope.org</a><br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""></blockquote><br class=""><br class=""><br class=""><br class="">--<br class="">Cameron Hummels<br class="">NSF Postdoctoral Fellow<br class="">Department of Astronomy<br class="">California Institute of Technology<br class=""><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__chummels.org&d=BQMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=hgcBC3x6dKFoTrmFmMYYbKNfiHZlGLKliIidd1LwmHI&m=kW1_fwfFTti2lMI7gO2kvyk104mlG15DxyGLw6CHK50&s=Gd_Q3yQTE3f8tc0bYjw48bvACWL12gSk3NUfjE9DANM&e=" class="">http://chummels.org</a><br class="">_______________________________________________<br class="">yt-dev mailing list<br class="">yt-dev@lists.spacepope.org<br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""><br class=""><br class=""><br class="">_______________________________________________<br class="">yt-dev mailing list<br class="">yt-dev@lists.spacepope.org<br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""><br class=""></blockquote>_______________________________________________<br class="">yt-dev mailing list<br class=""><a href="mailto:yt-dev@lists.spacepope.org" class="">yt-dev@lists.spacepope.org</a><br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""></blockquote><br class=""><br class=""></blockquote><br class="">_______________________________________________<br class="">yt-dev mailing list<br class=""><a href="mailto:yt-dev@lists.spacepope.org" class="">yt-dev@lists.spacepope.org</a><br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""><br class=""></blockquote>_______________________________________________<br class="">yt-dev mailing list<br class=""><a href="mailto:yt-dev@lists.spacepope.org" class="">yt-dev@lists.spacepope.org</a><br class="">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org<br class=""></div></div></blockquote></div><br class=""></div></body></html>