-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Prepare 0.23.3 #9817
Prepare 0.23.3 #9817
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
We're also going to pull #9819 (after backport) into this release too. |
|
||
This code is essentially copy-pasted from commutative_analysis.py. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably make this line a code comment rather than public documentation haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh yeah... I can do that instead. I at least added a line above to avoid this text being used as the summary in the TOC.
Matthew's also thrown #9828 into the mix (post backport). That's the end of the story, though - we're not putting in any more after that. If necessary, we'll do a 0.23.4. |
Don't worry about #9828 if the release is all ready and waiting on that to merge and backport; there is no reason to block on it. A local user can just cast locally before calling the methods so it's not critical at all. I only mentioned it if things were still in progress and we could include it in the release. But it's fine for 0.23.4 (or just 0.24.0) as it's very low priority. |
Eh, I think we can still fold it in today. |
Should now be up to date to include 36807d1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, I didn't check the unchanged release note files so I'm assuming those look ok.
They looked good to me when rendered locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this looks sensible thanks. There's places where it's clear our documentation isn't great (CommutationChecker
, for example), but now is not the time nor place to fix it.
Summary
Details and comments