Conversation
386bbdd to
a8fbdae
Compare
mejrs
left a comment
There was a problem hiding this comment.
I was thinking about keeping acquire_gil around for a while longer, because I think a lot of people still use it, and maybe use it in ways that aren't trivial to rewrite using with_gil. That said I also don't like having both around, so 👍 to this.
I think this would be worth it considering how much tricky/unsafe code can be removed or get reduced visibility due to this, especially the drop order checking which could inject unexpected and intermittent panics into downstream code. |
|
I think what I'll do is add a migration guide entry for removal of |
|
@davidhewitt I would like to help here if I can as I would very much like to see this merged for the reduction in complexity it allows. Would you mind if I tried to rebase this and add another commit for the migration guide entry? |
|
Please do, at the moment when I get a scrap of time I'm trying to finish off #3029 |
a8fbdae to
52d7eb1
Compare
|
I added an entry to the migration guide but admittedly the example is quite artificial. Please have a look and let me know if this is sufficient for our purposes. |
davidhewitt
left a comment
There was a problem hiding this comment.
Looks great to me, I agree it's artificial though it clearly demonstrates what a problematic pattern might be and how Python::with_gil would be used instead.
…lures when this was installed on-demand in the MSRV Clippy job.
52d7eb1 to
3343d68
Compare
|
bors r=adamreichold,davidhewitt |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Since #2980 starts a breaking change for 0.19, let's also clean up all 0.17's deprecations.
I've removed
Python::acquire_gilin its own commit, as that was a reasonably chunky removal.