Moved previous version changelogs to separate files; Added a Script to generate future changelogs #1138
Conversation
original taken from Numpy This will be used to generate changelogs. Signed-off-by: Naveen M K <[email protected]>
naveen521kk
force-pushed
the
changelog-gen
branch
from
02c9c3d to
4f98343
Compare
naveen521kk
added
pr:easy review
|
With the addition of this I think we ought to also make a short standardized way of naming PRs. If every PR starts with an identifier like "Bugfix:" or "New Feature:" or "Refactor:" etc. I think it would help us to be more organized (this could either be directly written in the PR template or have a link to our wiki or docs with the information for naming). To show a bad example, my own PR was poorly named because of the redundancy between "Bugfix" and "Fix." "Bugfix: Fix WebGL renderer hot reload on Windows" I am open to some disagreement on this naming convention though because while it does help organize, it is more complicated. Another thing I thought of recently is that imo one of the changelog headers should be documentation. I think it makes more sense for new or changing documentation to be listed under its own section. |
|
I agree with @WampyCakes, especially because we have opted to NOT use towncrier which would force the PR author to create their own "fragments" with the appropriate extension. I'm wondering if we can make this script better by retrieving the label information from the PR so that we can still automatically organize the PRs into sections without changing the workflow of the PR author. That said, I think that can be done in a future PR. This LGTM! Just remember to add this to the GitHub Wiki (or eventually the contributing section in Sphinx docs see #1137 ) |
|
I like the idea of using labels. Before merging, an assortment of labels that could be chosen by the merger could go a long way in putting PRs under the correct headers. Obviously this would not be a sufficient replacement to well-named PRs though. |
|
I wonder if we could add a GitHub test to make sure the title has a tag as we do with black. We would need a standard, but we should open a new issue for that. |
|
I think what we can do is add a section in the PR template which the script picks as a changelog seems to be a better idea. thoughts? But it would too hard for this to work with the current release. |
|
I think a better method is to use GitHub's API. For instance, for this PR In this manner, the sections can be organized by the appropriate labels. If the PR has multiple labels, we would need to decide which label to put it under. We already have the PR #s from this script so this wouldn't be that difficult to implement given a python json parser. But for a future PR. We would need more labels though, borrowing some from NumPy I think these would be enough (some we already have): API: an (incompatible) API change |
|
Let the labels be like, everything with a common prefix say
A PR can be marked one or more than one, and also PR authors will write a description in PR body starting like ## Changelog
<!--changelog-starts-->
```rst
[Changelog goes here]
```\
<!--changelog-ends-->where we will look for the two comments and pick the code block and paste it in the changelog, under appropriate sections as labels say the section. Thoughts? In this way we would explain users more through changelog. |
I don't like this because forcing the
Using GitHub's API seems best imo. A PR will ALWAYS have a title (it should be descriptive enough if the reviewers follow the checklist) and can easily have labels added. If the title/label is edited after the PR is merged, the changelog can still be automatically regenerated using the updated values from the API. |
I think you meant the devs. I would say it is fine to do so because they are the people who wrote the code and know what it does. Maybe Reviewers can help in writing them.
In that case, it is about just falling back to the PR title.
I would say it is much necessary, RST format is used everywhere in docs and also in docstrings. MD should be fine if necessary changes are made, if that's an issue.
I originally liked the idea of towncrier, but it doesn't work as expected sadly :(
Doing this also needs to use the Github API. I am just adding extra info to your idea. Use the labels to organise into sections and have some info or detailed changelog more than just the title. |
Yes, I meant the PR author/dev.
Okay, I'm starting to like the idea more because of the fallback, but I still think it should be optional.
While I agree it is very useful (if not necessary), GitHub Flavored Markdown would be better because PR authors won't have to manually verify their RST is formatted properly outside of GitHub. Until #1137 adds the documentation to the website, this doesn't much sense to do with RST because some contributors don't realize there is a GitHub Wiki page for our documentation -- instead they've used a plugin (#1064) or just copied the documentation of similar items already in the docs.
I was wondering why they didn't when I was working on towncrier last week. I know towncrier hasn't had a latest release in a very long time so it has to be downloaded from latest/put into the pyproject.toml as a URL. I didn't look much further into towncrier, but I was still able to generate new fragments for different sections with
In general, this can work and the extra information from the detailed changelog/description is useful if the title isn't enough space, but it would need testing to make sure it works at release time since we just need it to be automatic. To make this easier, maybe it shouldn't be RST or GFD, but just a simple paragraph. Eitherway, we could still do the label organization for this release. We still have ~1 week to label the merged PRs for this release. I think it'd be too much work to add in paragraph descriptions for the merged PRs, or test that it properly gets the changelog paragraph description properly/fallsback. |
BTW, for the towncrier I already spend some amount of time naveen521kk@0789af3 in writing PR desc.
We can use some kind of project to convert those markdown to rst format.
We have a link to the wiki in the PR template.
The final generated changelog is broken you can try it out naveen521kk@0789af3 |
|
On that note, it took ~15 minutes to go through the already merged PRs and add the appropriate labels to those missing them. |
We do, but it's under the reviewer section -- I think people see the part that says " |
|
I will work on this tomorrow or the day after. |
|
I'm currently working on it, will push if I get a neatly formatted rst -- otherwise my changes will be in https://github.com/jsonvillanueva/manim/tree/changelog for you to work from |
|
Was able to generate this changelog after tagging locally a v0.5.0 I still have to label a few of the "unlabeled" PRs but there's a few things I'd like to implement before merging this:
|
original taken from Numpy This will be used to generate changelogs. Signed-off-by: Naveen M K <[email protected]>
There was a problem hiding this comment.
Thank you very much for all of your efforts here, I like the generated output a lot!
I only have one remark: when stating it as "X people have contributed to this release", I feel that also reviewers should be included in that list -- after all, reviewing code is also a contribution.
We could also reword it slightly ("X people have authored changes for this release" maybe?), that would be more explicit in some sense, but if I could choose, I'd prefer including reviewers.
|
I also really like the level of automation we will reach with this pr, thanks a lot for your effort. |
|
@behackl This looks relevant though: https://pygithub.readthedocs.io/en/latest/github_objects/PullRequest.html?highlight=pullrequest#github.PullRequest.PullRequest.get_reviews @kolibril13 |
If you'd rather work on #1013 that's perfectly fine. I have some time today and can investigate both of these issues. |
By all means, go ahead! As for the actual change: The logic for where the authors are credited would need to be scratched/done differently -- I think it should be based on the :class: Would still need to investigate why #1075 is not in the list though |
|
I've pushed a commit which makes the script also get the list of reviewers. It works locally, and I'm open for feedback. If someone with a less experimental setup (I currently have to exclude support for Python 3.6 manually on my system, because otherwise a too low version of I looked into collapsable sections for a bit (we could use something like https://pypi.org/project/sphinxcontrib-details-directive/, but would then have to deal with formatting I think; haven't tried it locally yet) but have to go and do some other things now. From my point of view, this PR could also be merged without support for collapsable sections. |
Codecov Report
@@ Coverage Diff @@
## master #1138 +/- ##
=======================================
Coverage 54.29% 54.29%
=======================================
Files 108 108
Lines 14947 14947
=======================================
Hits 8115 8115
Misses 6832 6832 Continue to review full report at Codecov.
|
| @@ -1,427 +1,138 @@ | |||
| ######### | |||
There was a problem hiding this comment.
I think we should keep the old changelog somewhere
There was a problem hiding this comment.
Indeed. We can maybe think about splitting the changelog into different files for different versions, but I am also against simply overriding the current file.
There was a problem hiding this comment.
This was my bad. I accidentally added these changes in 7d9d608. Feel free to revert this file for now.
I think changelog.rst should just be a .. toctree:: linking to the version of the generated changelog (e.g. 0.5.0-changelog.rst) and the older version should be renamed and/or split into multiple files? What do you guys think?
There was a problem hiding this comment.
I think that the script, writing it to a file, I think it should be better than we print it on screen and ask the release managers to place it accordingly.
What I think is to create a folder called changelog, have this as changelog-0.1.0-0.4.0.rst and newer changelogs will have release-notes-0.5.0.rst. Thoughts?
There was a problem hiding this comment.
For the sake of consistency, I'd rather split all of the previous section into separate files. But otherwise I am all for it. 👍
There was a problem hiding this comment.
I reverted the changelog (95b02a4) in case we'd like to revisit it, but I've also gone ahead and done two small things (be5baa7):
- Split the previous section into separate files
- Made the script output
{cur_release[1:]}-changelog.rstby default (remove thevfrom the git tag/revision and place it in the appropriate directory under a:reversed::glob:... might want to set a depth.
There was a problem hiding this comment.
remove the v from the git tag/revision
What do you mean by this?
There was a problem hiding this comment.
be5baa7#diff-c461d965355498ee3bd152eda8723432a8d81b2bf09246f0cc8bf38c96d406a4R162
I'm not actually renaming the tag/changing the versioning system
There was a problem hiding this comment.
I think the current version of the script is good to go; it will certainly be better to use this script than generate the changelog manually for the next release.
If all open discussions are resolved and no further concerns are raised, I'll merge this in a few hours. (Or if someone else wants to do in the meantime: please feel free to do so.)
Further (substantial) improvements can be made in follow-up PRs.
|
Thanks for the work here @jsonvillanueva and @behackl |
behackl
changed the title
jsonvillanueva
changed the title
7 participants
Originally taken from Numpy
This will be used to generate changelogs.
Motivation
Fixes #1091
Tried towncrier but it wasn't useful enough that the Numpy(which we initially looked at) doesn't use it.
This script quickly generates a changelog for whoever is making the release.
This will require some amount of manual work and isn't as simple as tagging a release on Github.
The script is well documented and I will update the instructions on the Wiki page later.
Testing Status
Works locally
Logs
Acknowledgements
Reviewer Checklist