Use towncrier for changelog generation
#6974
Conversation
Pull Request Test Coverage Report for Build 2686032977
💛 - Coveralls |
requirements_test.txt
Outdated
| @@ -11,3 +11,4 @@ pytest-xdist~=2.5 | |||
| # Type packages for mypy | |||
| types-pkg_resources==0.1.3 | |||
| tox>=3 | |||
| towncrier~=21.9 | |||
There was a problem hiding this comment.
I would include the dependency here so that contributors can actually use the towncrier create command.
|
The check on the pr number is possible in GitHub actions because GitHub actions have access to it. That's why I wanted to change that. If towncrier is doing the check for the change description another way I'd prefer keeping the consistency with the previous changelog. I don't mind either solution as we'll be able to navigate from issues to PR anyway as they are linked. (Some reference are for PR when no issues were created already so we'll have to handle the disambiguation pr/issues to link to te right thing in the documentation whatever we choose) |
This comment has been minimized.
This comment has been minimized.
As far as I understand the integrated I currently use the stem of the news fragment file to construct a link to the issue in the changelog: |
|
I think we can split the check for the news fragments in two parts:
This way contributors can see as early as possible if the created news fragment has the correct format. And if they forget to add one, the CI will notice and inform them. |
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] Create a news fragment with `towncrier create <IssueNumber>.<type>` which will be | ||
| included in the changelog. If necessary you can write details of offer examples on how | ||
| the new change is supposed to work. |
There was a problem hiding this comment.
| - [ ] Create a news fragment with `towncrier create <IssueNumber>.<type>` which will be | |
| included in the changelog. If necessary you can write details of offer examples on how | |
| the new change is supposed to work. | |
| - [ ] Create a news fragment with `towncrier create <IssueNumber>.<type>` the type can be "new_check", | |
| "removed_check", "extension", "false_positive", "false_negative", "bugfix", "other", or "internal". | |
| (For example ``towncrier create 1234.false_positive``). If necessary you can write details or offer | |
| examples on how the new change is supposed to work. |
There was a problem hiding this comment.
It seems the list can be generated from towncrier.toml, hardcoding is enough imo, I'll update the suggestion
script/bump_towncrier.py
Outdated
| @@ -0,0 +1,38 @@ | |||
| # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html | |||
There was a problem hiding this comment.
Probably something that should be done in "python3 script/bump_changelog.py {new_version}" There's quite a bit of cleanup to do there with the adoption of towncrier ! We still have the script on the maintenance branch for 2.14 so almost everything can be removed (I think we only need to create the new file for minor automatically ?)
There was a problem hiding this comment.
I agree. I was a bit hesitant because of the "DO NOT MODIFY DIRECTLY" comment. 😄 That's probably not relevant any more / not relevant until we might decide to use towncrier in android as well?
Reading the release.md documentation, we have to handle two different invocations of tbump:
- Invocation for a release version (without a
-devX):
1.1 Update theversionintowncrier.toml
1.2 build the final documentation - Invocation for a dev version:
2.1 Update theversionintowncrier.toml
2.2 Update thefilenameintowncrier.toml
2.3 Create a newdoc/whatsnew/MAJOR/MAJOR.MINOR/index.rstand link that in the parentindex.rstfiles' toctrees
Maybe we should do the tbump related things in a separate PR, to keep this one reviewable.
There was a problem hiding this comment.
Hmm, right. Astroid is behind now and has a lot less activity than pylint. Last time I did astroid's script first, with the intention to use the same in both but pylint has different need. I don't mind keeping the old release and script routine in astroid as there is no whatsnew in astroid so no duplication for contributor.
There was a problem hiding this comment.
Did you have time to look at "python3 script/bump_changelog.py`` @DudeNr33 ? (Sorry I'm answering a conversation that is at the top of the page it's unclear with the review).
There was a problem hiding this comment.
Not yet unfortunately. I'll try to get to it in the next days.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Sorry guys for letting this open for so long. Check out the current docs build here. I added newsfragments for all changes currently included in the 2.15 changelog until now, and can do a last update before we merge. |
There was a problem hiding this comment.
Thanks a lot for all the work here @DudeNr33!
I left some initial comments while I'm trying to wrap my head around what towncrier is doing for us 😄
This comment has been minimized.
This comment has been minimized.
|
Thanks for the feedback, Daniel! As it is now already 3 weeks since we initially discussed
I will incorporate the rest of your feedback tomorrow. 😊 |
|
There was a problem hiding this comment.
Thanks for the clear answers @DudeNr33
Some (final) comments, but this looks really promising!
This comment has been minimized.
This comment has been minimized.
script/check_newsfragments.py
Outdated
| if verbose: | ||
| print(f"Checked '{file}': LGTM 🤖👍") | ||
| return True | ||
| print(f"{file}: does not respect the standard format 🤖👎") |
There was a problem hiding this comment.
Could we add something about what the format is here?
There was a problem hiding this comment.
Sure, this would be helpful. Instead of just printing the cryptic regex I included a short text with placeholders.
DanielNoord
added
Skip news 🔇
|
One last question from my side: Beside that I think all open issues are addressed now. I'd just make sure that I double check if I have transformed all changelog entries into news fragments just before we merge. |
|
Probably something to do with broken caches. Sadly Github doesn't allow removing caches through a nice interface and you can only do so by sending several Bumped the cache to a number that we have never used and that seems to work. All tests are running now. |
It did not work two days ago, unless we already used |
That might have been too long? CI passed 😄 |
|
I regret rebasing on the 6 tests commits I did now 😄 Tried, 6, 7 , 8, 10, and a big number... I guess we'll never know it's too hard to check in https://github.com/PyCQA/pylint/actions?query=branch%3Atowncrier. Thanks for fixing this @DanielNoord ! |
|
Thanks Pierre and Daniel! |
I'd prefer a |
Thanks for the hint, @DanielNoord!
I tried that with a toy repo and it looks like that should do the trick, but step 4 makes me want to re-assure myself if this is really correct. 😄 |
|
I would add a rebase on origin/main before 4 (that should be painless). To be safe in 4) you can |
Yes, this is what I was thinking of! |
|
Thanks a ton you two! I always learn something new here. 👍 |
|
@DudeNr33 Note that the first commit in this history can be avoided by a Always happy to help, I tend to learn quite a lot from your PRs as well. |
Yeap, confirming that, I don't comment on it but I often learn of new, more elegant way to do thing when reviewing. Great job @DudeNr33 this is a huge new feature, I feel like I handled changelog conflict 10% of the time for the last 5 years, I'm glad it's over 😄 I'm going to try an alpha/beta release with it and see how it goes 👍 |
doc/whatsnew/2/2.15/index.rst(ordoc/whatsnew/2/2.14/full.rstif the change needs backporting in 2.14). If necessary you can write
details or offer examples on how the new change is supposed to work.
and preferred name in
script/.contributors_aliases.jsonType of Changes
Description
Draft for using
towncrierto generate the release notes.Feedback is welcome.
Things that need to be discussed:
In this comment @Pierre-Sassoulas had the idea to reference the PR number rather than the issue number. Is this still the way we want to go? As it is not possible to know the PR number before creating the PR itself, this would mean one needs to always create and push the news fragment after drafting the PR. I find that inconvenient.Reorganize the changelog and the way it's displayed in the doc #6688 (comment)
Things I still need to implement:
towncrier.tomlwith atbumpscript - I would like to wait for feedback first before doing that.Things that have to be done before merging: