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

Automatically add dates to release notes #8001

Merged
merged 6 commits into from
Apr 25, 2024
Merged

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Apr 22, 2024

This adds a small Sphinx extension to add the date to the titles of the release notes, both on the page and tables of contents.

Before After

I based this on @jaraco's nifty https://github.com/jaraco/rst.linker, and I've added you as co-author on the commit, hope that's okay?

This PR works by getting the title of pages matching releasenotes/\d+\.\d+\.\d+", then looking up the timestamp from a git log call, and then adds "(yyyy-mm-dd)" to the title.

There's some things in rst.linker which make it not suitable to use here, and I think it might not be worth generalising it further:

Some notes:

  • When it can't find a Git tag, this PR adds "(unreleased)" instead of "(yyyy-mm-dd)". This is for when the next version isn't released or tagged yet (see 10.4.0)
  • This means, when building from a fork, it will show "(unreleased)" next to any release if you haven't fetched the upstream tags.
  • Similarly, it will show "(unreleased)" if you build the docs from a zip or sdist.

I don't think these are problems? For forks, git fetch upstream --tags will grab them all; I added it to Read the Docs for forks builds. For sdist builds, we could return early if a .git directory is absent.

@aclark4life
Copy link
Member

aclark4life commented Apr 22, 2024

Wooo ✨ I was thinking about adding dates there too recently so, thank you!

@jaraco
Copy link
Contributor

jaraco commented Apr 22, 2024

Thanks for looping me in.

I've added you as co-author on the commit, hope that's okay?

Perfectly fine.

There's some things in rst.linker which make it not suitable to use here.

I'd be open to extending rst.linker if that makes sense... or even just exposing some of the functions for customized re-use. No obligation, though.

  • It names the output filename (files).rst and deletes the original filename.rst

Minor correction - it creates a new file filename (links).rst containing the links as a temporary, implementation internal artifact, and it deletes that file after the rendering is done. It doesn't touch the original file.

It adds the date underneath the title. I considered this, but think it's better for us in the title

I made this choice intentionally in order for the anchors to be predictable (#v69-5-1). With dates in the title, the anchors will include the dates. That's also why I use v-prefixed versions (a title must begin with a letter to be used as a stable anchor). I can see how that's of far less value when there's a file per release.

@hugovk
Copy link
Member Author

hugovk commented Apr 22, 2024

I'd be open to extending rst.linker if that makes sense... or even just exposing some of the functions for customized re-use. No obligation, though.

Thanks, I think the end result here is quite compact.

Minor correction - it creates a new file filename (links).rst containing the links as a temporary, implementation internal artifact, and it deletes that file after the rendering is done. It doesn't touch the original file.

Ah right, it was just my hacky WIP when I first looked at modifying it :)

I made this choice intentionally in order for the anchors to be predictable (#v69-5-1). With dates in the title, the anchors will include the dates. That's also why I use v-prefixed versions (a title must begin with a letter to be used as a stable anchor). I can see how that's of far less value when there's a file per release.

Thanks for pointing that out. Yeah, that definitely makes sense for https://setuptools.pypa.io/en/stable/history.html#v65-6-1, and doesn't really matter for https://pillow.readthedocs.io/en/stable/releasenotes/9.4.0.html.

cmd = ["git", "log", "-1", "--format=%ai", git_version]
try:
out = subprocess.check_output(
cmd, stderr=subprocess.DEVNULL, text=True, encoding="utf-8"
Copy link
Member

@radarhere radarhere Apr 24, 2024

Choose a reason for hiding this comment

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

Why are both text and encoding used? From testing locally and from the docs, I would think only one is necessary.

https://docs.python.org/3/library/subprocess.html

If encoding or errors are specified, or text is true, file objects for stdin, stdout and stderr are opened in text mode using the specified encoding and errors or the io.TextIOWrapper default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Funny story - originally, only text=True was used. Then later, when PEP 597 introduce encoding warnings when the encoding wasn't specified, indicated to include the encoding. Eventually, the need for encoding= will be once again unnecessary (python 3.14 maybe), so we'll want to fall back to text=, but until then, both are there. Maybe text= could be removed in the interim, but I'm kind-of inclined to leave it there as it's the primary signal.

Copy link
Contributor

@nulano nulano left a comment

Choose a reason for hiding this comment

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

Looks good, and even works out-of-the-box on Windows (at least on my system)!

Use split instead of datetime
@radarhere radarhere merged commit 4241836 into python-pillow:main Apr 25, 2024
52 checks passed
@hugovk hugovk deleted the dater branch April 25, 2024 05:12
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.

None yet

5 participants