Skip to content

Update development practices #734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Apr 8, 2025

This updates development practices to:

  • Require that one review is required for all PRs. This is to prevent developers merging there own PRs, which I have been very guilty of in the past. Although it can be frustrating not getting a review, I think it's probably worth tipping the balance in favour of waiting and getting a review here.
  • Require that new minor releases are announced for a week before happening. cc @jakirkham

@@ -278,6 +277,10 @@ hard-and-fast rules, e.g., it is fine to make a minor release to make a single n
feature available; equally, it is fine to make a minor release that includes a number of
changes.

When making a minor release, open an issue stating your intention so other developers
know that a release is planned. At least a week's notice should be given for other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
know that a release is planned. At least a week's notice should be given for other
know that a release is planned. In most cases, at least a week's notice should be given for other

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reluctant to un-tighten the language here - I think for minor releases (as opposed to micro releases with bugfixes) a week's notice is reasonable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find these sorts of black/white constraints to introduce unnecessary friction. We don't want to get into a place where a minor release needs to go out but someone objects to its release because the announcement was only 5 days prior. I'm looking for directional guidance here, not hard and fast rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "should be given" already indicates that almost always it should be respected, but there's room in exceptional circumstances to bypass the reccomendation? At least that's the existing language in this document, and it's not prefaced by "in most cases" elsewhere.

@jhamman
Copy link
Member

jhamman commented Apr 8, 2025

Thanks @dstansby for opening this up. A few other observations that we may want to address here or elsewhere:

  • the release workflow described here is quite outdated. We should not need to be creating tags locally and pushing them to GitHub. There is an opportunity to move closer to what Zarr-Python does here.
  • taking that further, it seems like we're missing a number of the core recommendations from SPEC 0008. I'm curious if folks think there are things we can take from this that would help avoid yesterday's situation in the future?

cc @zarr-developers/python-core-devs


A note about yesterday's release. It was a bit of a bummer to have a breaking release in numcodecs force a release in zarr-python. That said, it wasn't that big of a disruption and is something that we can hopefully use to motivate improvements in our process here. Much better to learn our lesson on relatively inconsequential mistakes than on big ones!

@d-v-b
Copy link
Contributor

d-v-b commented Apr 8, 2025

broadly speaking I think the dev tooling in numcodecs could use some love. see #697. I have been satisfied with everything we are doing over in zarr-python w.r.t. CI and the developer environment. Copying that to numcodecs wherever possible would probably be a good use of effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants