Improved changelog to be automatically managed by towncrier #885
Conversation
|
From your experience with the tool, what are the benefits of separate files? A summary of the change and the type of change should already be in the PR/commit so there will be a manual copy and paste action that seems to break single source of information. If we have a PR/issue template saying that you should provide the type of change and a summary of that change we would avoid that step. The information towncrier needs could be extracted from GitHub. How should commits not related to an issue or PR be handled. Maybe not a big issue. Most of that tends to be things not very interesting to the user. |
IMO, I think it's just easier to put the news in separate files with the issue/PR# and type of news in the name in a volatile location that is automatically removed by towncrier when a release is cut. This way, if the PR submits a relevant change for end users you can say "please add a news/changelog entry with a quick summary of your change" that can be reviewed before the PR is merged.
I see your point, but generally, a PR/issue has more information than a user cares to know about. What it looks like y'all currently do is scrub the commit history since a previous release for notable changes and add them to e.g. This change effectively does the exact same thing, however, it is done immediately by the author or a maintainer when it's realized something is relevant for end users and doesn't require extra time during a release to dig in further because everything has been reviewed and approved before a merge to master. The difference, though, is that the repetitive steps are automated by a popular tool for other Python projects: content for changes is copied to the release notes, issues are categorized into sections to make finding information easier, and links are added to the origin of the change if a user needs to investigate or if they're curious about the internal changes. The new workflow is basically this:
My understanding of your release process is that this change adjusts when the news is created for the release notes to be before a PR is merged such that the news can be created by a maintainer or reviewed by them instead of scrubbing the commit history and trying to remember important details or reviewing code changes/comments again to see if there's something relevant that commit messages don't mention. My argument against your last point is that this does not break the "single source of information". Towncrier will automatically add a link to the issue/pr based on the file name and categorize it in the release notes based on the type of change. The goal of this is to change the focus of the release notes to be user-facing instead of developer-facing. In essence, the files in TLDR: check out keep a changelog, that'll be way more concise than what I've described here.
I don't think you would want that, though. Again, my strong opinion about changelogs is that they're for conveying relevant information to users, commit history and issue/pr titles/descriptions/comments are not focused that way. They are for developers, not users. Like I mentioned in #881, I don't want to see any commit history in the changelog as a user if it doesn't affect me because it's distracting. If you deprecate a feature, introduce a breaking change, or add new features to the public API, I care about seeing those. But, I don't want to see anything about a CI failure that was fixed or a fix to lint issues because they don't affect me. I go to a changelog to see what actually changed from my perspective as a user and to determine if I need to open an issue if something broke, for anything else I'll go look at closed issues if I care for developer commentary. I did add a misc section for anything that might be relevant to users, but at least now it's categorized into sections that will hopefully make it easier to find important information. In any case, towncrier doesn't force you to comply with opinionated rules about managing a changelog. It simply has a convention to automate some of the repetitive steps of doing so, so I figured it'd be easy enough to submit a PR to let someone else's code take the burden off maintaining it and also provide some personal insight into how I, as a user, use your release notes.
Agreed, I haven't been able to think of an example where a change doesn't have an issue/PR# that is also something the user cares about. However, towncrier does support this by using an "orphan prefix" (which defaults to |
| "-TEWanb", | ||
| "html", | ||
| Path(__file__).parent.parent / "docs", |
There was a problem hiding this comment.
I believe this was a bug and not sure how it worked before... There were errors in git bash where the command line args would have something like WindowsPath("path/blah/...")
There was a problem hiding this comment.
Probably just needs to be wrapped in str().
|
@GlenNicholls I completely agree that Towncrier is a step forward compared to what we have today but it may be possible to take it one step further. Today, we don't have any templates for issues and PRs and that is also something we need to improve. Quite often we have to ask the basic questions about what version is used, what simulator, what OS, how can I reproduce etc. A typical template for a PR starts with a short summary describing what bug is being fixed or what user value is added. There are also labels to categorize the PR. In many cases the summary + the label + the PR number = the news file if the label is "news worthy". The tricky part which may kill that idea is if you have a PR adding X such that Y becomes obsolete. The PR summary would be "Added X and deprecated Y" but this might deserve two news files. One for the addition and one for the deprecation. Do you use it like that or can news files have several types? Anyway, PR-generated news files would be a separate step before running Towncrier and can be added later if it turns out to be valuable. For now I think it's sufficient to keep it in mind. For example, Towncrier types should be aligned with PR labels. We should also create a PR template that explains that a news file is needed before the PR can be merged. We also need a job that fails if a PR is created without such a file. |
I agree. I know GHDL and other projects can have certain fields populated such that a GHA can be kicked off to reproduce the problem based on a MWE. However, I think that this is a very large effort outside the scope of the issue. I'm willing to help in that area, but I don't know that it is a good thing to attempt that here.
I'm still not sold that a PR description is appropriate as user news. Most PR descriptions I see might be empty or contain lower-level details. It's all valuable when you're debugging, but not for simply seeing what's changed or getting hints about where to look when something breaks. Maybe it would make sense to have a required section when there is a "news worthy" or "user-facing" label that would populate changelog entries. However, IMO it's the same amount of work requiring the same review; regarding what tradeoff is there, I'm not sure. With that said, it could be at least automated, but if there's a CI check for news entries when there's a specific label, then I don't think it makes sense to complicate or deal with the complexity of the GitHub hosting side of things.
News files cannot have several types. That feature might be in their roadmap, not sure. But, in that case, you would create two news fragments, e.g.
100% agree. Are the current types satisfactory? In the above case, I think it makes sense that a PR would allow for multiple labels and CI would check that all labels have created a news file for that issue with those types. For the labels (assuming you are satisfied with the current types and the section verbiage), the following make sense to me:
EDIT: IMO, the label should have info that it is for the news. The other reason I like the above is that they're easy to parse to ensure that the |
|
@LarsAsplund sorry for the spam, ran into a bunch of issues setting the matrix since GHA doesn't support evaluating the matrix in a conditional To test this PR for the automated checks for the news, you can add the |
|
It seems that this PR/branch gathered a bunch of modifications and tests! I suggest we split it into smaller PRs, to more easily review and merge. I would start with just "changing the current flow to use towncrier instead of manually writing one rst file for each release". We need to release v4.7.0. Using the same approach as in the last 4y, I would go through v4.6.0...master and would manually create two files: the rst to be included in the docs and the same in markdown syntax to be used in the description in GitHub Releases. Let's change that: let's create a subdir in @GlenNicholls, in the examples you provided, the "change" files are written in In parallel to adding change files for the next release, I would suggest having a PR to:
Next release
------------------
.. include:: release_notes.inc
That will simplify the maintainers' effort to Also in parallel, we can address archiving release notes of previous versions. That is:
That will work in master. It will fail when creating a release because the corresponding file does not exist. In this PR, the logic is removed from
So, the maintainer who is creating a release needs to either copy a file or execute a command with an option (which is never used in CI). And the release script checks that such manual action took place. I would defer the other topics mentioned (and implemented) in this PR until we merge these 2-3 PRs I proposed above. Using labels, triggering CI when changing them, automatically requiring files with specific names to be created,... I feel that's too far away from the reality of this project. We never asked contributors to add a line to the release notes, and we don't typically require the docs to be updated in the same PR where technical changes are done. I believe we should start with using towncrier and assuming that maintainers will add change files after they merge/push some relevant modification. I.e., let's assume everything will be post-mortem, but let's transition to a culture of not doing everything just before a release, but sometime between merging (including as part of the merge) and before a release (the closer to the former, the better). We can later enforce it to be done at the time of merging only, after/if maintainers prove to be capable of doing it ourselves. We don't need towncrier to do that. We could do it by just adding lines to @GlenNicholls do you want to address any of those PRs? I can start with the first one; to prove that @LarsAsplund and you both have valid points, I can try automatically generating one change file for each commit between v4.6.0 and master. Then, we can see how much needs to be removed and how much is missing. I expect not all the relations with PRs/issues to be explicit in all commits. That will let us know how much effort we need as a community to use towncrier properly. |
Yeah... because the original idea had a scope change request in the PR.
That was the goal. Providing VUnit with a way to mange a changelog that was user facing, not developer facing. With that said, I see no issue with creating a rst file per release. You add a rst file whenever there's something relevant to the user and towncrier handles deleting it when it's released... To be clear, adding new files to the release section of the repo is the whole point of towncrier. At the end of the day, my compliant is that a changelog reporting commit history is absolutely, 100%, useless. I can see commit history in commits in GitHub, GitLab, etc.. What I expect to see in changelogs is relevant information for a user like API changes, REST API differences, etc. Specifically, I care about user-facing changes in changelogs, not developer information.
This PR already does that - removes developer notes and commit messages from user facing release notes and allows a single ReST file for anything useful created in
Again, I don't see the benefit. 99% of the current release notes are not useful... Let's take 4.6 as an exmaple an examine what's acutally useful for the average user. Examine the below and let me know what you would find useful as an end user? I found exactly ONE change that I found useful....... As you'll see, theres 1 (ONE) change that's actually useful for me...
Why waste time or release notes to cover internal notes? This PR fixes that by providing a template to fix developer vs user facing release notes. If you don't want it, that's fine. However, I would appreciate a thorough plan if you want any more time from me to fix this integration. |
I understand. Yet, I would like to discuss technical/implementation details about the various features. Discussing all of it here would be hard to follow. Since we can split it into smaller chunks, I believe we'd better do it step by step.
I understand and agree with the motivation. I would personally not be so minimalistic, but that's not relevant. I'm good with the selection of "relevant" changes proposed by you, Lars or anyone else. My concern is not conceptual, but pragmatic. We have ~175 items which we need to filter/select (see https://github.com/VUnit/vunit/pull/918/files) before we can execute towncrier to generate the summary/changelog/notes. Until we don't do that, we cannot know if the fields/categories/templates suffice to capture the knowledge we want to share with users. For instance, I believe that bumping dependencies might be a category itself.
I believe we have different points of view about what towncrier can do and what we want it to do. I agree with creating changelog/release notes that are user facing, rather than developer facing. I agree that towncrier can help by letting us write one file per change, using the filename to encode a backref to the issue/PR and the category/type. From there on, I don't think we want towncrier to delete anything, nor to manipulate/modify files/releases. Instead, we want it to assemble the individual change files and produce a single block of text that we can a) include in the sphinx documentation and b) include as the description of the Release in GitHub. That's mostly because letting towncrier do anything else involves unnecessarily increasing the complexity to start using it. We might want to improve that later, but at the moment we need a markdown file that we can copy and paste into the release description, and ideally we can include that same file in the sphinx documentation.
I meant/mean "let's create a subdir in docs where we put one for each meaningful change [since v4.6.0]". I'm trying to reduce the technical complexity of adding towncrier and instead focus on creating the content that towncrier needs to process/use. From this PR, I want to pick:
I would leave other modifications aside, for now:
By picking those changes, and having change files for the modifications since v4.6.0, we can start using towncrier and create the relase notes (changelog) for v4.7.0 with it.
There is no benefit, and I don't want to waste a time I don't have. Yet, we need to create the change files for towncrier to generate any meaningful output. The sources of knowledge we have are the following:
This is the first time we have issues/PRs milestoned; in all previous releases it needed to be checked by looking at the dates. This is to explain where we are coming from and where we are now; which we need to know if want to draw the path to where we want to be. Please, let me know how do you suggest we create a summary of 1, 2, 5, 10 meaningul changes from those 175 items. My understanding is that one or multiple humans need to do that, because no one generated files for towncrier in the last year. So, we (you, me, whoever interested in using towncrier) need to go through the change files in https://github.com/VUnit/vunit/pull/918/files and elimite all the ones that are not meaningful, apart from rewording the ones that are to be kept. Naturally, I'm open to any suggestion to achieve the same result with less effort.
I am all in with using towncrier. The thorough plan is:
|
|
FTR, with towncrier's option |
The Goal
This PR removes the manual changelog/release notes management by using towncrier. User-facing "newsfragments" live in a specific location (
docs/changelog.d). For VUnit, it is set up such that the reST news files are of the form<issue #>.<type>.rstwhere the type can be one of (all config inpyproject.tomland template format indocs/changelog.d/template.rst.j2):breaking- breaking changes, section header in release notes will beBreaking Changesbugfix- A bug has been fixed, section header will beBug Fixeschange- A backwards compatible change has been made, section header will beChangesdeprecation- A feature is deprecated, section header will beDeprecationsmisc- Misc. changes that may be relevant to users, section header will beMiscellaneousNote that all
type's are supposed to be user-facing, but, themiscis for everything else that might be notable. For example, this PR adds news for #881 like881.misc.rstthat states how the changelog will be changing for future releases.The nice part about this is a maintainer can just tell someone submitting a PR to also add a newsfragment instead of scrubbing commit history during a release. There might need to be some adjustments to the verbiage to make it digestible for end users, but it will hopefully take some maintenance burden off the core maintainers.
What Changed?
This PR removes the complicated release notes process and validation. Instead, developers add newsfragments for notable, user-facing features as single files to a directory Towncrier searches.
towncrier buildwill then construct the changelog based on those files, stage those files for removal, and the user commits those changes.The other feature added is that the
py*-docsplans add a draft version to the release notes, but the release plan will add the final version. Towncrier in draft mode writes to stdout with what it will be inserting, so when the code does that it just writes to a file instead that only gets included if it is not being built for a release.Lastly, the old changelog release notes file contents have been added to
docs/release_notes.rstand that file is now tracked by git.tools/release.pywill also add and commit everything relevant for this process.Outstanding Questions
Can we remove the
Commits since previous releaseandCommits since last release? Those IMO are not relevant in a changelog since it's developer facing for debug. I've never seen that before in release notes and it is straightforward for a developer to figure that out with git directly or in the GitHub UI. As a compromise, I changed those for future releases toCommits since this release, but I still don't think it's all that useful to link in an area where most people reading it would open an issue instead of attempting to fully debug a regression. There will be complexity associated with supporting the two links I mentioned and I just don't think they're all that valuable for the end user.Are the selected news types (
breaking,misc) good or should they be changed?Any other examples you want to see other than what's below?
How is documentation uploaded to RTD? For releases, the user will need to install towncrier, sphinx, and sphinx-argparse for the
release.pyscript to run. Because of the awkwardness with the old manually managed release notes,release.py createwill write generated HTML torelease/docs.Final TODO's
scripts/release.pyworks properly with these changes. I have not tested this yet as I don't know the release process as looking at GHA, it looks like releases are cut manually instead of detecting them fromvunit.__version__.news:breaking,news:bugfix,news:change,news:deprecation,news:misc) andCI checks to ensure newsfragment is created if label is present.<checkout>/release/docs. The action is at https://github.com/VUnit/vunit/blob/master/.github/workflows/docs.ymlon.label, so not sure if metadata change will... Need to add an expected label to this PR to test this. Another one is linking a forked branch PR to an issue that contains these labels. I don't have permission to apply labels/link issues to test this.Examples