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

Drop CHANGES.rst in favor of GitHub Releases #198

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

jeffwidman
Copy link
Member

I started to cut a new release but realized it's a bit painful that we are currently maintaining both a CHANGES.rst file and also tagging releases in GitHub releases UI.

For a busy project, maintaining a dedicated changelog makes sense... but the reality is this project is in maintenance mode, and we the maintainers are more likely to cut releases when they're easy/low friction. Since GitHub very nicely has the "Auto-generate-release-notes" button, let's just use that.

I considered copy/pasting the results from that to CHANGES.rst, but even that feels like a waste of time.

@@ -1,224 +0,0 @@
Changes
Copy link
Member Author

Choose a reason for hiding this comment

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

I chose to outright delete CHANGES.rst for simplicity... this is a bit heavy-handed, I also considered simply renaming it to CHANGES_ARCHIVED.rst, but decided against it for three reasons:

  1. We've been maintaining changes in the GitHub releases UI for a while, so there's already a recent changelog that goes back as far as anyone who's trying to actively use this project would really need.
  2. Some tooling (Dependabot, RenovateBot, etc) may look for *change* filenames when auto-populating changelogs... so keep life simple and force them to look for the changelog URL from setup.cfg and not let it accidentally pickup a CHANGES_ARCHIVED filename.
  3. If we leave the file around, then at some point in the future we'll eventually delete it... life is simpler if we just nuke it now. It's still in git history for anyone who really needs it.

Open to discussion if other folks feel strongly that we should keep it.

Copy link
Contributor

@nickjj nickjj Jan 17, 2023

Choose a reason for hiding this comment

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

Hi,

I think your reasoning for dropping the file is sound. The only comment I have is around "hidden expectations". As an end user I usually dart directly for a changelog file in the repo to get a glance of what's either released or coming soon. If I don't see that file, it's an extra step to check the releases. Some folks may not know it exists there.

From a technical standpoint it couples things to GitHub directly but I think in the grand scheme of things this is a non-issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

You both make good points. I don't feel strongly either way, and think that deleting the file now is probably best. I don't think it will be too difficult to see changes with the new method, and people will find what they need.

@jeffwidman
Copy link
Member Author

Note that this is a stacked PR, ideally #197 is merged first and then this should auto-update w/o having merge conflicts.

@davidism
Copy link
Member

davidism commented Jan 17, 2023

I personally don't like the "generate notes" feature, I prefer a well formatted, deliberately written message for each PR, rather than using whatever random commit messages everyone has used. And since the generated notes @ mention authors, if they get copied anywhere everyone keeps getting pinged, such as an update bot copying release notes, or some project that gets manually mirrored to github as many separate forks (which happened with Jinja last year, that was my one experiment with the feature, I think it was linux that did it).

I also feel like change notes are "intrinsic" to a repo, not an extra feature, they get included in sdist builds for example. So moving them out of the repo itself into a GitHub-specific feature adds a little more vendor lock-in to the project. It looks like the changes aren't included in the docs, so at least that's not disappearing.

All that said, neither Thief nor I are really the maintainers here, we're just in the background as Pallets support, so this project should do whatever its active maintainers and users find most convenient.

Base automatically changed from fix-remaining-urls-to-pallets-eco to master January 17, 2023 16:16
I started to cut a new release but realized it's a bit painful that we
are currently maintaining both a `CHANGES.rst` file and also tagging
releases in GitHub releases UI.

For a busy project, maintaining a dedicated changelog makes sense... but
the reality is this project is in maintenance mode, and we the
maintainers are more likely to cut releases when they're easy/low
friction. Since GitHub very nicely has the "Auto-generate-release-notes"
button, let's just use that.

I considered copy/pasting the results from that to `CHANGES.rst`, but
even that feels like a waste of time.
@jeffwidman jeffwidman force-pushed the switch-to-using-github-auto-generated-releases branch from aa653a8 to bd346a0 Compare January 17, 2023 20:05
@ThiefMaster
Copy link

why not keep both? having to go though many releases is very annoying, even more so when there are prereleases in between... and I'm not even going to start taking about using commit messages for the changelog - i have yet to find a project where this doesn't at least add some noise that's irrelevant to an end user of that project

@nickjj
Copy link
Contributor

nickjj commented Jan 17, 2023

I feel similar to David around preferring a hand written changelog for the messaging itself. Git provides us a commit history if you want the low level gory details. Mainly as an end user I'm glancing it to see what's new, what changed, deprecations or potential bug fixes.

For the last couple of years I've been defaulting to https://keepachangelog.com/en/1.0.0/ as a style for changelogs. It's a bit more structured than a free for all changelog and could make it easier to manually fill out since types of commits or PRs can be bucket into a specific thing. It's more systematic while also giving a human touch.

For a project that gets a handful of PRs every couple of months, the extra effort to hand write a changelog doesn't feel like a bottleneck or deterrent. It's mainly ensuring each PR is well tested and safe to merge, then there's the mechanisms of cutting the release itself such as ensuring certain files are updated, building and pushing it to the test PyPi repo, double checking it works "for real", then cutting a proper release.

@jeffwidman
Copy link
Member Author

I'm totally cool with whatever, I just don't want to spend my limited time right now on updating a full-blown changelog.

If other maintainers want to do that, then go for it. Just right now it's a friction point to me cutting a new release, so I'm far less likely to do it compared to just clicking a button, making a few tweaks on the auto-generated result, and calling it good.

I'd also be okay copy/pasting the output from it into a changelog file... but historically on other projects I've found that I sometimes forget to do that.

Anyway, we do need to get a release cut--if someone else wants to handle writing the changelog and cut a release, please do so.

@davidism
Copy link
Member

davidism commented Mar 2, 2023

it's a friction point to me cutting a new release

This is why in Pallets I enforce adding a changelog to each PR before merging it, rather than trying to remember and collect everything at the end.

I'd recommend taking the cost for this release to get the changelog up to date, then make sure pull requests have change entries from now on.

@Dosenpfand
Copy link
Contributor

Is an update to CHANGES.rst currently the only blocker for a release? If so, I would volunteer to update the document.
Does every merged PR need an entry? Or should minor changes, e.g. typo fixes be skipped?

@davidism
Copy link
Member

Changes that affect the use of the library, such as bug fixes, changes, and features, should be logged. Changes to docs, CI config, etc. should not.

@Dosenpfand
Copy link
Contributor

Thanks for the quick reply! I created a PR at #200.

Copy link
Contributor

@macnewbold macnewbold left a comment

Choose a reason for hiding this comment

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

I think it's a good time to move ahead with this. Thanks @jeffwidman !

@macnewbold macnewbold merged commit d036021 into master Nov 21, 2023
10 checks passed
@macnewbold macnewbold deleted the switch-to-using-github-auto-generated-releases branch November 21, 2023 15:26
@greyli
Copy link
Member

greyli commented Nov 26, 2023

It's not the way to go. The end user only cares about the major changes (manually added to the changelog file) instead of every commit generated by GitHub's generate button.

@asteinlein
Copy link

It's not the way to go. The end user only cares about the major changes (manually added to the changelog file) instead of every commit generated by GitHub's generate button.

Very much +1 to this and what @davidism has said, but seems the maintainer here chose a different route. I'm just now going over all our dependencies for upgrades (which is how I landed here now), and I love projects that maintains a changelog. Makes the job of considering upgrades so, so much easier. Auto-generated from commits is absolutely no help at all; I could just as well look at the commit history for that.

@ThiefMaster
Copy link

ThiefMaster commented Nov 26, 2023

Exactly his. Please consider bringing back a changelog.

I know many people just blindly upgrade all their dependencies or don't even pin them. And fine, whatever floats their boats.
Just like @asteinlein I always go over the pip-compile output to see what will be upgraded, look at changelogs (especially if it's a significant version bump), and possibly test some parts of the code manually if I think a change may affect it (and not be covered with unit tests).

Having to go through a commit log makes this very painful, and GH releases are not much better: I now need to click through all the individual releases, instead of having a single page with just the relevant changes (I don't care if e.g. the codebase got formatting changes, some new linter, or if docs were updated).

The draft 0.14.0 release proves my point. From have a very quick over the changes (not looking at the actual PRs):

# Changelog-worthy changes
Submodule to use https protocol after unencrypted git proto deprecated by @natecollins in #175
Add ARIA role to toolbar for accessibility improvement by @natecollins in #174
Permit scrolling for content panels by @natecollins in #173
Expand HTTP codes on which the toolbar will be displayed by @natecollins in #176
Fixed scrollbar issues by @caffeinatedMike in #182
updated to work with flask 2.2+ by @christopherpickering in #183
Flask-SQLAlchemy 3 compatibility by @Dosenpfand in #186
fix: migrate from deprecated flask.Markup to markupsafe.Markup by @miettal in #203
fix: use urllib.parse.quote_plus and drop werkzeug.urls.url_quote_plus by @miettal in #207
fix: drop response.charset because charset deprecated by @miettal in #206
No need to specify custom default value if key not found by @jeffwidman in #210

# Internal/infrastructure/docs - not relevant for users
docs: Fix a few typos by @timgates42 in #180
Fix outdated docs links by @jeffwidman in #187
Point at new location of django-debug-toolbar by @jeffwidman in #189
Fix Flask SQLAlchemy quickstart link by @frafra in #196
Point URLs at pallets-eco/flask-debugtoolbar by @jeffwidman in #197
Set up GitHub actions to replace Travis by @greyli in #215
Fix tox and GitHub actions settings by @greyli in #217
Fix lint issues and lint config by @greyli in #219
Fix the test for basic app by @greyli in #221
Use standard Python gitignore file by @greyli in #220
Drop CHANGES.rst in favor of GitHub Releases by @jeffwidman in #198

# Unsure w/o checking the changes (but likely internal)
Replace deprecated threading.currentThread with threading.current_thread by @hugovk in #179
Remove deprecated charset property from process_response content crafting by @dadavec in #211
Remove the use of before_first_request by @greyli in #218

Useful changes, but completely irrelevant for someone just using the library. Even some of the changelog-worthy changes could probably be grouped as "Fix compatibility with flask/werkzeug version X"...

@macnewbold
Copy link
Contributor

I'm new here, and don't have a strong opinion either way. My main bias is toward progress and forward motion, and if this helps make it more likely that we'll get releases out more often, I was leaning that way. Personally I like that the generated results acknowledge the contributors, but I'm not stuck on that. If we have a plan/policy about what to include in the generated results or not, I don't think there'd be a problem cutting stuff out from the generated results if we want to streamline more, like @ThiefMaster did. I'm not a fan of redundant work (for anyone), and I'm not super concerned about vendor lock in, as it doesn't seem like anyone is planning on migrating all the pallets-eco away from GitHub, especially since we're tailored to a lot of the other tools (PRs, automated testing, etc.) as well. Personally I don't see a big difference between https://github.com/pallets-eco/flask-debugtoolbar/releases and a Changes file inside the repo itself, but I understand that others have some strong feelings about it.

I'll defer to whatever the other maintainers want to do, and have no issue if we want to undo the merge of this PR.

@jeffwidman
Copy link
Member Author

jeffwidman commented Dec 7, 2023

Basically this comes down to whether someone raises their hand and volunteers to maintain the changelog on an ongoing basis.

For context, I last worked professionally with Flask over 5 years ago. The main reason I'm a maintainer here is I have a soft-spot in my heart for Flask and I didn't want to see this project die--I know how frustrating it is to open a PR against a useful project only to get ignored by the maintainers, so I stuck around as a maintainer to help keep the lights on. But it has been mostly as a I have a few moments here and there I'll merge PR's to unblock folks. I actually do this for quite a number of small open source projects that are essentially in maintenance mode where no one else has been merging the bugfix/deprecation PRs.

As I indicated above, I am totally fine if someone else wants to maintain the changelog, but it's not something I'm personally going to spend time on, as I don't see the value for such a low-activity repo. We did a "big bang" release to catchup with the last year and a half of changes, and the resulting auto-generated release notes takes only a minute or two to skim: https://github.com/pallets-eco/flask-debugtoolbar/releases/tag/0.14.0

I do find it interesting that I put up this PR a year ago, a few folks chimed in against it, and since it wasn't something I felt strongly about (other than I wasn't going to spend my time on it), I let it languish. But no one else did anything to help either so the result was no release until @macnewbold and @greyli showed up just recently to get things going again. Doesn't it seem like a release with a sub-optimal changelog is better than no release at all?

As an aside, I'm also not personally a fan of the "require users to add a changelog entry"--I've seen that in action, and it tends to result in merge conflicts when a maintainer swoops in to merge a bunch of PR's. Frequently the user's changelog entry isn't actually that useful and needs rewording, which may not be possible if they haven't allowed maintainers to edit their PRs. Easy enough to fix on an individual basis, but over time it once again adds up to another source of friction.

Again, while I have an opinion, it's not actually strongly held at all, so if anyone else wants to do it differently, more power to you! Just let's be sure you're committing to helping out going forward and not merely giving your drive-by opinion.

@nickjj
Copy link
Contributor

nickjj commented Dec 7, 2023

We did a "big bang" release to catchup with the last year and a half of changes, and the resulting auto-generated release notes takes only a minute or two to skim: https://github.com/pallets-eco/flask-debugtoolbar/releases/tag/0.14.0

After having seen the auto-generated release notes, this is better than I initially expected.

It's not super formal, the messages are kind of inconsistent, nothing is really categorized but you know in the end I don't think any of that matters. There's ~25 bullet points and like you said, it's skimmable in a minute or 2.

The upsides are there's zero overhead, you get the gist of what changed, who made the change and where the PR lives to dive into more details. As an end user of the extension I don't think I'd be upset with this outcome. Everything is there that I need to know about to get a rough idea of what it will be like to upgrade versions.

Also maintainers have the option to click the edit button and manually adjust the notes. GitHub implemented this nicely because it copies in the markdown for you to edit so if you truly wanted to spruce up how it's displayed then you can do it. This gives us the best of both worlds where it's auto-generated but we have the option to improve it later (but probably won't need to).

If someone is poking around the repo hoping to see a changelog related file I think if we really wanted to we could solve this with GH Actions. Once GH Actions produces a release it wouldn't take a lot to prepend a line to the file with a link back to the GitHub release.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants