A review of a PR against mmap

As I mentioned elsewhere, I’m trying ease myself back to core dev contributions. I’ve submitted a PR against the mmap module, given that it was a later development by eryksun of something I’d worked on back in 2008(!)

Although I could technically merge it myself, it’s my first contribution under the GH workflow – and my first of any kind for a long while. So I’d appreciate it if someone cared to look over it, whether for the basic approach (wording etc.) and/or the technicalities of the patch, which is entirely Windows-based.

I’ve tried to follow the current devguide guidelines but – amusingly – there’s really too much information now, rather than too little :slight_smile:

If there’s no particular push-back I’ll revisit in a few days to merge the PR.

Thanks in advance

Mechanically your PR looks fine. I left a couple comments about the contents of it.

Main thing to be aware of when you merge is to change the PR number in the commit title from (#29213) to (GH-29213). Otherwise, we won’t know later on that it refers to a GitHub number. There’s a bot that will yell at you if you forget, but it’s already merged by then so it can’t be fixed.

Thanks for that. Hopefully I’ve now addressed all the points.

Thanks to everyone who reviewed / helped. That’s been merged now

1 Like