Skip to content
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

Add commentary to release instructions to drive discussion #1614

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

Conversation

aragilar
Copy link
Member

@aragilar aragilar commented Aug 2, 2020

This should close #1300 (once it's done) - people should feel free to push changes (I figured it was easiest to discuss this via a PR).

This currently has a lot of comments in the rst file, hopefully we can replace this comments with actual text so everyone knows what the release process is.

In terms of things that have been done in #1300, we have the API docs updating automatically now via github pages.

I think the main thing is working out how to handle the wheels (can rever have multiple stages?).

@codecov
Copy link

codecov bot commented Aug 2, 2020

Codecov Report

Merging #1614 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1614   +/-   ##
=======================================
  Coverage   85.80%   85.80%           
=======================================
  Files          17       17           
  Lines        2247     2247           
=======================================
  Hits         1928     1928           
  Misses        319      319           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b6aaea...f85d1d0. Read the comment docs.

@@ -21,8 +26,17 @@ that everything you need to perform the release is correctly installed
and that you have the correct permissions. All rever commands should be
run in the root level of the repository.

..
rever currently is set to upload to PyPI, which will likely cause issues as
we need to upload the sdist and wheels at the same time
Copy link
Member

Choose a reason for hiding this comment

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

Do we? I thought it was possible to upload them separately, so long as each file is only uploaded once.

Copy link
Member Author

Choose a reason for hiding this comment

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

#1300 (comment) was what I was going off, I think the issue is if there's no wheel for the latest version, pip tries to build from source, which errors, meaning the install fails. I guess it depends on how long it takes to get and upload the wheels (the longer it is, the more people get a build error - I imagine a hour or so it would be fine, longer I'm not so sure).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, gotcha. I was thinking of limitations on uploading to PyPI, but it makes sense that we don't want a release uploaded without wheels for any length of time.

@aragilar
Copy link
Member Author

aragilar commented Aug 2, 2020

I'm not exactly sure how MacPython/h5py-wheels#11 affects this, we may need to change how the wheels are built for 3.0.

@aragilar
Copy link
Member Author

aragilar commented Aug 2, 2020

I wonder if something like this could work: we do everything up to the PyPI upload via rever, which instead uploads the sdist somewhere (TBD, ideally the place we upload the wheels) and pushes the new version to the h5py-wheels repo. The wheels for mac/linux get uploaded via CI (based on the sdist we uploaded), and then we upload the wheels from Christoph Gohlke. Then there's a final script which does the copying over of the sdist/wheels to PyPI.

..
I'm guessing we want this to just push the tag to GitHub.
Things that could differ from matplotlib in this step
* Empty commit
Copy link
Member

Choose a reason for hiding this comment

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

The empty commit is because Matplotlib uses versioneer to inject the version which injects the names of all of the references to the tagged commit into the tarball so if you have both the tag and a branch and then later just a tag the hash of the tarball will change.

I'm guessing we want this to just push the tag to GitHub.
Things that could differ from matplotlib in this step
* Empty commit
* Tag signing (rever appears not to have that) - probably not needed
Copy link
Member

Choose a reason for hiding this comment

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

I'm +.5 on doing the signing.

Things that could differ from matplotlib in this step
* Empty commit
* Tag signing (rever appears not to have that) - probably not needed
* The format for the github release (we may not care as long as it's
Copy link
Member

Choose a reason for hiding this comment

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

For Matplotlib we just use the body of the tag message.

* The format for the github release (we may not care as long as it's
consistent)

We should also work out what need to happen for zenodo
Copy link
Member

Choose a reason for hiding this comment

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

This should be triggered by the creating the release on github.

@takluyver
Copy link
Member

My experiences so far for 3.0.0rc1 (#1695):

  • rever check worked OK, but I got emails from Github warning me about a deprecated authentication API.
  • I ran subsequent steps one by one (rever -a authors) instead of just letting it go, to keep control over the process.
  • authors complained about one Github username (from PR Update the docs of meta-data cache config functions #1576). I don't really know why, nor why the check we run in CI didn't already complain - it's otherwise quite picky about the .authors.yml file. I added the username by hand and it was happy.
  • version_bump silently missed the rc1 from the tuple that ultimately defines h5py.__version__. I think the regex is not quite right. Fixed manually.
  • changelog
    • Called the file 3.0.0.rc1.rst - we'd ideally like it to be 3.0.rst.
    • Didn't put a title on the page ("What's new in h5py 3.0")
    • Didn't add the new file to whatsnew/index.rst
    • Got some entries in the wrong section - e.g. my recent addition in Get processor architecture at compile time #1691 went under 'Building h5py', although I had put it under 'Bug fixes' in the PR.
    • Left some <news item> entries in the changelog.
    • Split 'building h5py' into two sections

Even if all that is fixed, I think the canonical release process should stop and let the maintainer check things manually at that point. No automated process can turn a bunch of little fragments into decent release notes by itself - you want someone to rearrange them, straighten out the wording, fix links, etc.

@takluyver
Copy link
Member

I continued stepping through the actions one at a time. tag, push_tag and ghrelease all seemed to work with no issues. The pypi action gave up with RuntimeError: password not in pypi section of /home/takluyver/.pypirc. That's quite true - my password is not in .pypirc, because twine & flit can both retrieve the password with the keyring library, allowing it to be stored more securely.

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

Successfully merging this pull request may close these issues.

Document release process
3 participants