changelog: remove git shortlogs #4497
Conversation
|
This looks really good to me. It's very clear how everything works and the usage of the diff in the changelog file itself is a good idea. |
c6428a2 to
92ca030
Compare
92ca030 to
52f0d27
Compare
52f0d27 to
1713d98
Compare
1713d98 to
d2becf1
Compare
|
This should now be ready for review. As mentioned, I did some test releases and it should be fine. I deleted these test releases, but if you want confirmation, I can push another test release again. I unfortunately made a mistake in #4503 and forgot about the wheels in the dist directory which are now also uploaded to 4.0.1 on GitHub. This PR would fix it again and would only upload the tarball (plus signature). There's also the option of replacing the streamlink-bot token with a simple GitHub actions token when creating a GitHub release. streamlink-bot has become obsolete and was only added in the past because some API OAuth token was needed to create releases or to push commits (in other repos). This is now fully automated via |
Yeah I'd prefer we no longer use this account for security purposes if we don't have to. I don't see any reason to support both. Edit: @bastimeyer were there any changes made since I looked at this a few days ago other than the change in #4503? |
This is the diff compared to the very first push So basically just a few clean-ups, bugfixes and minor changes in regards to the API key. Nothing functional. Considering that there could be more problems with 4.0.x, I'd suggest not merging this yet. It's just an improvement for the readability of the changelog anyway (with the added contributors list). |
- Completely rewrite GitHub release script
- Remove --dry-run and make it implicit when no API key is set
- Remove --api-key and read from RELEASES_API_KEY or GITHUB_TOKEN
- Add --template, --changelog, --no-contributors and --no-shortlog
- Refactor GitHub API stuff
- Update release template logic and use Jinja2
- Get list of contributors (with support for commit co-authors):
GitHub highlights usernames in releases, which this is meant for
- Generate git shortlog automatically
- Always close open file handles
- Update release template based on Jinja2 logic
- Update release CI job
- Install Jinja2
- Upgrade Python to 3.10 and setup-python to v2
- Fetch entire repo
- Don't upload wheels to GitHub releases
d2becf1 to
bc4be51
Compare
|
One final update:
Test release for demonstration purposes: |
| env: | ||
| RELEASES_API_KEY: ${{ secrets.RELEASES_API_KEY }} | ||
| run: ./script/github_releases.py "${STREAMLINK_DIST_DIR}"/* | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
there is no secrets.GITHUB_TOKEN in
https://github.com/streamlink/streamlink/settings/secrets/actions
or is it hidden?
There was a problem hiding this comment.
This rewrites the changelog and GitHub release body. The goal was to get rid of the git shortlog in the changelog.md file, as it bloats up the entire file by a lot and is mostly unnecessary. I've replaced each shortlog with a simple link to the git commit history between the previous tag and the current one on GitHub, where commit diffs are just one click away, which is much better in most cases.
The changelog is used in a couple of different places and removing the git shortlog has different implications:
A link to the commit history with access to diffs makes more sense than a shortlog
The commit log and diffs can be read locally
A link to the commit history with access to diffs makes more sense than a shortlog
Removing the git shortlog could be a problem here, but I don't think it's too big of an issue
A link to the commit history with access to diffs makes more sense than a shortlog
The new release script automatically generates a shortlog in a addition to a new contributors list with GitHub account names for automatic highlighting, so nothing will be changed here. Having a shortlog on the releases page makes sense, because the changelog often doesn't include everything and refers to the shortlog for minor plugin changes/fixes.
This doesn't split the changelog in the docs into multiple pages like I originally intended, as it would require writing a new Sphinx extension. Removing the shortlogs however already solves most of the navigation / legibility issues of the current changelog page in the docs.
I haven't verified these changes in my test repo with a test-release yet, but the dry-runs all looked fine. Opening this as a draft for now, because it won't be part of 4.0.0 anyway.