[yt-dev] Analysis module review process

Britton Smith brittonsmith at gmail.com
Tue Dec 29 15:07:10 PST 2015


I agree that the number of approvals is less important than ensuring that
PRs come with appropriate tests and docs.  If the PR is a change to an
existing analysis module, we should just make sure the relevant devs are
identified and that they sign off on it.  If it's something brand new, I
think 2 or 3 people is probably still a good idea.

On Tue, Dec 29, 2015 at 4:44 PM, Matthew Turk <matthewturk at gmail.com> wrote:

> Ideally, with the barrier to acceptance reduced, the reviews that come
> in may be deeper and more thoughtful ... and less along the lines of
> "Can somebody just hit the approve button?"
>
> On Tue, Dec 29, 2015 at 4:41 PM, John ZuHone <jzuhone at gmail.com> wrote:
> > +1 for lowering the barrier. At a minimum, tests should pass, and new
> tests
> > should be encouraged if possible. Docs as well.
> >
> > Not too worried about 1 or 2 approvals. We could try 2 at first to see if
> > this helps things out.
> >
> > On Dec 29, 2015, at 5:39 PM, Cameron Hummels <chummels at gmail.com> wrote:
> >
> > As long as the code being changed is local to an analysis module and not
> > used by other parts of the main yt codebase, yes, I'm all for dropping
> the 3
> > reviewer requirement to 2.  1 might be pushing it though.
> >
> > On Tue, Dec 29, 2015 at 2:38 PM, Matthew Turk <matthewturk at gmail.com>
> wrote:
> >>
> >> That's precisely what I had in mind.
> >>
> >> On Tue, Dec 29, 2015 at 4:36 PM, Nathan Goldbaum <nathan12343 at gmail.com
> >
> >> wrote:
> >> > On Tue, Dec 29, 2015 at 4:29 PM, Matthew Turk <matthewturk at gmail.com>
> >> > wrote:
> >> >>
> >> >> [+-][01] on reducing review overhead for analysis modules?
> >> >
> >> >
> >> > Does this just mean reducing the number of PR reviewers before we
> merge
> >> > pull
> >> > requests? I'd be ok with that, but just want to clarify what you have
> in
> >> > mind.
> >> >
> >> > _______________________________________________
> >> > yt-dev mailing list
> >> > yt-dev at lists.spacepope.org
> >> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >> >
> >> _______________________________________________
> >> yt-dev mailing list
> >> yt-dev at lists.spacepope.org
> >> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
> >
> >
> >
> > --
> > Cameron Hummels
> > NSF Postdoctoral Fellow
> > Department of Astronomy
> > California Institute of Technology
> > http://chummels.org
> > _______________________________________________
> > yt-dev mailing list
> > yt-dev at lists.spacepope.org
> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
> >
> >
> > _______________________________________________
> > yt-dev mailing list
> > yt-dev at lists.spacepope.org
> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20151229/33bbb0ef/attachment.htm>
-------------- next part --------------
_______________________________________________
yt-dev mailing list
yt-dev at lists.spacepope.org
http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org


More information about the yt-dev mailing list