Patrick
Patrick
I believe some of the comments I've written don't make sense, although it's hard to tell in what way they don't make sense and how to improve them, so I...
Ah, the "Compare" button isn't as useful this time because it also includes all the changes from `master` that were included. I'll list out the changes I made: - At...
I realize that my note about calling `ensure_neighbor` instead of `neighbor` was unnecessarily verbose for what is already a long and dense explanation, so I replaced it with the `(and...
> Please rebase for clarity. Done! Thanks for reviewing the other PRs so quickly!
I think this is ready for another review, as I've fixed the worst readability issues. The lint build step will fail until #456 is merged. EDIT: I just realized that...
@Ralith: I finally got around to addressing all of the PR comments. For re-reviewing this, I believe the main thing to focus on is `NodeBoundedRegion` and its tests. Most new...
> All outstanding comments have been addressed, and I don't have the bandwidth to internalize the actual math, so I think this has been reviewed about as well as it's...
Unfortunately, due to https://github.com/rust-lang/rust-analyzer/issues/21478, rust-analyzer currently doesn't like peer_traverser.rs. We can work around this by no longer using `VALUES.len()` and instead creating a separate `COUNT` field on `Side` and `Vertex`...
> For now, let's just replace the `VALUES.len()` call with its literal, and leave a TODO citing the RA bug. Compilation will still fail if the value changes and we...
> In that case, my recommendation would be to add a commit (or follow-up PR) that replaces `len()` calls with TODO-bearing literals as needed, which we can then easily revert...