Skip to content

Conversation

@behackl
Copy link
Member

@behackl behackl commented May 1, 2021

Changelog

  • Added generated changelog
  • Bumped version to v0.6.0

@behackl behackl added the release A tracking issue for changes expected for a release label May 1, 2021
@behackl behackl added this to the Release v0.6.0 milestone May 1, 2021
@behackl
Copy link
Member Author

behackl commented May 1, 2021

I would still like to get #1430 and #1431 in. Please everyone @ManimCommunity/core check out the generated changelog (as soon as RTD finished the build)!

* Abhijith Muthyala
* Adam Ryczkowski +
* Alex Lembcke +
* Anton Ballmaier +
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Anton Ballmaier +
* Anton Ballmaier

I have authored a patch in 0.5 as well. Other names may be wrong as well?

Copy link
Member

@jsonvillanueva jsonvillanueva May 1, 2021

Choose a reason for hiding this comment

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

Can you link the PR? This will help to debug the dev_changelog.py script.

Copy link
Member

@jsonvillanueva jsonvillanueva May 1, 2021

Choose a reason for hiding this comment

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

Nvm, I found it:
image

The issue here is that your name changed between v0.4.0 -> v0.5.0 and v0.5.0 -> v0.6.0 from AntonBallmaier to Anton Ballmaier with a space -- our script works by pulling the author name and comparing author names from the set of past contributors to current contributors with the git shortlog command. I suppose we could do it based on a combination of email/user, but maybe there's a better approach using GitHub's API.

The parameter `path_arc` of :class:`~.Transform` now works with the `.animate` syntax

* `#1364 <https://github.com/ManimCommunity/manim/pull/1364>`__: Added :meth:`~.Mobject.match_points`
- Added :func:`~.Mobject.become_points`, which transforms the points, positions and submobjects of a Mobject to match that of the other while keeping style unchanged.
Copy link
Contributor

Choose a reason for hiding this comment

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

CC @Darylgolden

Suggested change
- Added :func:`~.Mobject.become_points`, which transforms the points, positions and submobjects of a Mobject to match that of the other while keeping style unchanged.
- Added :meth:`~.Mobject.match_points`, which transforms the points, positions and submobjects of a Mobject to match that of the other while keeping style unchanged.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

can you change it in the PR title, then behackl would render this again?

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've already taken care of it. 👍

@behackl
Copy link
Member Author

behackl commented May 1, 2021

I fixed some more broken formatting and fixed a few links; I'm more or less fine with the current state of this. I'll be away for a few hours, everyone with write access can re-run the script though (or push changes to the changelog directly).

@Darylgolden
Copy link
Member

Also, can I ask how the list of contributors who reviewed PRs is determined? I tried to review several PRs this release, even though I don't have write access.

@jsonvillanueva
Copy link
Member

jsonvillanueva commented May 2, 2021

Also, can I ask how the list of contributors who reviewed PRs is determined? I tried to review several PRs this release, even though I don't have write access.

https://github.com/ManimCommunity/manim/blob/master/scripts/dev_changelog.py#L92

If you go to the Files changed tab there's a green dropdown button that allows you to make Reviews. I don't think comments are included in the GitHub API request.

@Darylgolden
Copy link
Member

Also, can I ask how the list of contributors who reviewed PRs is determined? I tried to review several PRs this release, even though I don't have write access.

https://github.com/ManimCommunity/manim/blob/master/scripts/dev_changelog.py#L92

If you go to the Files changed tab there's a green dropdown button that allows you to make Reviews. I don't think comments are included in the GitHub API request.

I'm pretty sure I did that several times.

@jsonvillanueva
Copy link
Member

jsonvillanueva commented May 2, 2021

I'm pretty sure I did that several times.

I'm not really sure what the issue is. We're not doing any filtering on whether the review is of type approves/request changes/comments and you do appear in at least one REST API query: https://api.github.com/repos/ManimCommunity/manim/pulls/1347/reviews

^This is what the github package is doing https://github.com/PyGithub/PyGithub/blob/master/github/PullRequest.py#L729-L739

@Darylgolden
Copy link
Member

I initially thought the issue was write access, but there are people without write access in the reviewers list too 😕

@Darylgolden
Copy link
Member

I also think @hydrobeam reviewed some PRs but isn't included too, such as at #1430

@kolibril13
Copy link
Member

Thanks for making the pr, I just saw that my name is also not there in the "have been reviewed" section.

@behackl
Copy link
Member Author

behackl commented May 2, 2021

For those not in the reviewer list: can you check whether you have a name set in your github profile? If not, then that's the issue. I think we can fix the situation by using the login name if no actual name is set.

@kolibril13
Copy link
Member

For those not in the reviewer list: can you check whether you have a name set in your github profile? If not, then that's the issue. I think we can fix the situation by using the login name if no actual name is set.

I did not set my name, but it is set now :)

@hydrobeam
Copy link
Member

I've also set my name 🚀

Comment on lines 19 to 20
* Aron
* Aron Fischer
Copy link
Member

Choose a reason for hiding this comment

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

I think both the names are the same right? @cobordism

@Darylgolden
Copy link
Member

I've set my (fake) name

@behackl
Copy link
Member Author

behackl commented May 2, 2021

PRs #1434 (obviously) as well as #1282 #1200 #1259 were not yet included in the list because their merge message have either been modified, or because they were merged manually/differently. I've added them now manually.

Also, I've changed the logic of how authors are determined -- it now also uses the GitHub API, the names in the authors and reviewers list should now be consistent.

Copy link
Member

@jsonvillanueva jsonvillanueva left a comment

Choose a reason for hiding this comment

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

I'm approving this in advance with these changes (Gotta make the first PR look great and remove the malformatted bullets); however, I'd like to mention that there could be a more consistent grammar/authoring of this text. As is, it appears to be written by various authors and in some cases there's redundancies between the PR title/body text. We'd be here all day doing nitpicks and the meaning is understood nonetheless... so I'd rather just release this considering the tradeoff.

@behackl
Copy link
Member Author

behackl commented May 2, 2021

I won't regenerate the changelog at this point, I think it is good to go. (Certainly not written uniformly, and some information might be redundant -- but I don't think there is a wide margin for improvement here.)

Pretty impressive work everyone, and thanks for reviewing the changelog! I'll merge and then release soon™️, if you have further comments or remarks, it's either now or never.

@behackl behackl merged commit e5a6307 into master May 2, 2021
@behackl behackl deleted the prepare-v0.6.0 branch May 2, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release A tracking issue for changes expected for a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.