Comments (5)
Thanks for starting this discussion @munkm !
I'm in favor of broadening the scope of PRs that require just 1 vote, particularly any infrastructure/maintenance PRs. I also agree with Matt that we need to somehow acknowledge the nebulous nature of any categories we define.
In terms of new features, where do new frontends fall? They can take quite a while to get through and since they're relatively self contained I could see a benefit to relaxing the approval requirement from 2 to 1 if some amount of time has passed. I'd still lean towards requiring 2 approvals since with the amount of new code it's helpful to have more eyes on it, but I thought it was worth bringing up.
from yt.
I generally agree with you here. I would like to respond to a couple specific points
new features probably should have 2 approving reviews to make sure that the feature is consistent with community needs/expectations.
While I agree that 2 reviews is generally better than 1, and that it'd be best to have as many reviewers as possible on features is ideal, I would argue that timing should also be considered. In some cases, it takes a very long time (several months, or even years) to get a single approval, and this can be crushing (when not a straight deal breaker) to the contributor. Not only does nobody expects to be in the queue for this long, keeping a PR alive also requires extra care and skills that I don't think we want to impose as another entry barrier.
This is, understandably, mostly due to a lack of dedicated time for reviewers to get through the backlog. In the absence of clear objection, like changes requested, or another maintainer clearly expressing that they'd like more time to review a change, I think it could be good to allow even new features to get in with a single approval after some to-be-discussed period of time.
But if we are just following NEP 29 then we have already established a drop schedule for most upstream packages, so is 2 approving reviews really necessary?
We may need more discussion on this. I know you co-signed NEP 29 on behalf of the project, and as the person taking care of enforcing python version dropping for the last 3 years, I'm absolutely in favour of following it. However, this is rarely discussed and I'm not sure every maintainer is aware that this document exists, or agrees to follow it. It also is now superseded by SPEC 0. Note that we are currently supporting a larger version range than this specification recommends : at the time I'm writing, we just dropped Python 3.8, and SPEC 0 recommends that we drop 3.9 in two weeks; we support numpy 1.19, while SPEC 0 already recommends dropping 1.21.
from yt.
In some cases, it takes a very long time (several months, or even years) to get a single approval, and this can be crushing (when not a straight deal breaker) to the contributor.
I am curious what examples exist for this past yt-4.0 or after the examples where the entire codebase was refactored and contributors were asked to wait until those PRs went in. The other example I can think of are PRs that were modifying important aspects of our community procedures. In my experience, the PRs that take a long time to go in are the ones that should have 2 reviews, as they either change a lot of the codebase or they require substantial community discussion because they change existing processes. Having 2 community members review (and ensure everything is satisfied) helps reduce the burden of a single reviewer catching everything.
I know you co-signed NEP 29 on behalf of the project
I didn't co-sign it on behalf of the project. I co-authored (and SPEC 0) it as a scientific python community member.
However, this is rarely discussed and I'm not sure every maintainer is aware that this document exists, or agrees to follow it.
Our community discussion about it happened after the NEP was published in #2307 . Do you have suggestions of a different venue where this type of conversation should happen so that the community knows we are following it?
FWIW the goal of NEP29/SPEC0 is not to have explicit version support drops, but rather that maintainers shouldn't be burdened to maintain support for those versions. There's no issue with us continuing to have Python 3.9 or numpy 1.20; our users should just expect that 3.9 support or Numpy < 1.21 is unstable and YMMV.
from yt.
... examples where the entire codebase was refactored and contributors were asked to wait until those PRs went in.
I was actually thinking of times where authors are not explicitly asked to wait, but reviews take a while to get in nonetheless. I'm not blaming anyone for it, but this feels like the typical case to me, so I'm worried that the developer time we have isn't always sufficient to meet our own standards.
I co-authored (and SPEC 0) it as a scientific python community member.
thanks for clarifying this ! I also didn't remember about #2307
There's no issue with us continuing to have Python 3.9 or numpy 1.20; our users should just expect that 3.9 support or Numpy < 1.21 is unstable and YMMV.
Sounds good to me, but do you know if we acknowledge this anywhere ? There's a historic support table in the installation guide but NEP 29/SPEC 0 are not mentioned.
from yt.
I think this is a really important discussion. I will say that I think the most important part is that we try to specify a few lines for where "user-facing" and "infrastructure" changes fall, and that we build into those grace for each other, and the knowledge that we may have different opinions about what constitutes those two categories.
from yt.
Related Issues (20)
- BUG: defect in polar coordinate handler breaks fixed-resolution buffer HOT 2
- BUG: accessing data in re-loaded sphere object errors HOT 1
- BUG: Order of Cylindrical Coordinates for Athena++ Incorrect HOT 6
- TST: `yt/tests/test_load_archive.py::test_load_archive` is failing HOT 1
- Possible bug: eps_writer doesn't retain formatting of SlicePlot object HOT 6
- Error while rendering SpectralCube with non-square face: IndexError while reading fields HOT 1
- pooch HOT 2
- TST: GAMER answer tests are failing HOT 6
- DOC/DEPR: eps_writer's documentation is misleading
- octree total gas masses improperly inversely scaling with nref HOT 11
- Off axis Slice Plot breaks when using derived field and default center HOT 1
- DOCS: adding/updating a list of external frontends (and extensions) HOT 3
- (not a BUG): flip_horizontal and flip_vertical with multipanel plots not working HOT 1
- TST: Upcoming dependency test failures HOT 3
- Question: expected behavior of center argument in slices of spherical datasets? HOT 3
- TST: Upcoming dependency test failures HOT 1
- TST: Upcoming dependency test failures HOT 2
- TST: incompatibilities with pytest 8 (tracking issue)
- bug/not implemented problem: annotate_contour does not work with GIZMO MFM output HOT 3
- Volume Rendering Multiple Fields with MPI HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from yt.