[yt-dev] Analysis module review process

[Nathan Goldbaum]

I agree. If we can identify maintainers for analysis modules, I think only
one additional approval besides the maintainer is fine. If the maintainer
proposes the PR, they should try to get at least one person to look over it
in detail before it gets merged.

On Tuesday, December 29, 2015, Britton Smith <brittonsmith at gmail.com> wrote:

> 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
> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','nathan12343 at gmail.com');>>
>> >> wrote:
>> >> > On Tue, Dec 29, 2015 at 4:29 PM, Matthew Turk <matthewturk at gmail.com
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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/41febd15/attachment.htm>
-------------- next part --------------
_______________________________________________
yt-dev mailing list
yt-dev at lists.spacepope.org
http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org