Enzo pull requests - need to deal with this for next public release!

6 views
Skip to first unread message

Brian O'Shea

unread,
Jan 13, 2016, 7:57:30 AM1/13/16
to enzo...@googlegroups.com
Hi folks,

We have three PRs that need to be accepted before we can have the next public release.  Two of these, Greg/Munier's cosmic ray PR [1] and Christine's kinetic feedback PR [2], now have two approvals and need a third (thanks to the reviewers of these!).  The other PR is Dave's MHD-CT cosmology fix [3], which currently has no approvals but will have at least one once I am done testing the new problem types that he has added, and thus will need two more.  

So, what's the problem?  My go-to reviewers are all extremely busy and a couple of the PRs are rather extensive, which makes people even less willing to review them.  (I totally sympathize with all of this, by the way.)  Now that most peoples' academic terms have started back up, it's even less likely that they're going to get reviewed in short order by somebody else.  It's not at all clear to me how this situation is going to resolve itself, since our pool of potential reviewers is rather small right now - part of the issue here is that lots of the community's go-to reviewers are authors of these PRs, and are somewhat reluctant to approve them due to perception of conflict of interest.

So, how do we proceed?  As I see it, we have a few options:

(1) Accept the PRs with two approvals each.

(2) Get additional volunteers until we reach the standard 3 approvals/PR.

(3) Issue the next public release without this code.

My preferred option is #1, since #2 seems unrealistic and #3 just defers the problem.  All of the PRs pass the test suite and have been looked at rather extensively, so I'm comfortable with approval.  Does anybody have any thoughts on this?

Thanks,
Brian

------------------



Christine Simpson

unread,
Jan 13, 2016, 8:06:01 AM1/13/16
to enzo...@googlegroups.com
Hi Brian,

I can take a look at the CR PR, if that will help.  

I would also like to add that I have been asked to review exactly 1 pull request since we have implemented the pull request system.  I don’t know why this is.  I guess I’m not monitoring the repo as much as others.  Should I be doing that more and volunteering myself?  I just mention this because I think the pool of potential reviewers may be larger than you suppose.

Christine

--
You received this message because you are subscribed to the Google Groups "enzo-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to enzo-dev+u...@googlegroups.com.
To post to this group, send email to enzo...@googlegroups.com.
Visit this group at https://groups.google.com/group/enzo-dev.
For more options, visit https://groups.google.com/d/optout.

Brian O'Shea

unread,
Jan 13, 2016, 9:32:16 AM1/13/16
to enzo...@googlegroups.com
Hi Christine,

Yes, it would definitely help if you could look at the PR!  Would it be possible for you to do it by the middle of next week?  Say, Wednesday?

In the case of this particular wave of PRs, I consciously did *not* ask you to review one because you were responsible for responding to updates on your kinetic feedback PR, and I thought it would be an unfair burden to ask you to both deal with (possibly significant) modifications to your own PR and also look at other ones.  Regarding earlier requests... I'm not sure.  We haven't had tons of PRs lately, and I have to admit that my process for asking people to be reviewers is a bit slapdash.

In general, it would be really helpful if you (or anybody else!) volunteered to review PRs, either by letting the current PR manager know or by adding yourself to a PR you're interested in.  I do suspect there are a fair number of people who *could* review PRs, but when I send out "help, we need reviewers!" emails they tend to be met with silence.  :-/  So, if people are generally willing, please let me know and I'll keep a list!

Brian
  






Christine Simpson

unread,
Jan 13, 2016, 11:00:14 AM1/13/16
to enzo...@googlegroups.com
Hey Brian,

I just spoke to Greg and I think the CR PR has a third reviewer.  I will try to take a look at the MHD-CT PR instead.

Christine

Greg Bryan

unread,
Jan 13, 2016, 3:02:27 PM1/13/16
to enzo...@googlegroups.com
I would be happy to go ahead with two approvals, although maybe we can get to three (in fact, the CR PR now does have 3 — thanks Dave!). I’ve also reviewed Dave’s MHD-CT changes and will approve pending a response.  I think Christine and Brian are also looking at Dave’s PR, so maybe somebody could take a quick look at Christine’s (which already has 2)?

Cheers,
Greg

Brian O'Shea

unread,
Jan 13, 2016, 5:08:40 PM1/13/16
to enzo...@googlegroups.com
Perfect.  Thank you!

Brian O'Shea

unread,
Jan 13, 2016, 5:08:51 PM1/13/16
to enzo...@googlegroups.com
Hi Greg,

That seems like a great plan to me.  Thanks for the suggestion.  Any volunteers to look at the kinetic feedback PR?

Brian

Reply all
Reply to author
Forward
0 new messages