On Thu, Aug 01, 2024 at 10:19:45AM +0200, Patrick Steinhardt wrote: > On Wed, Jul 31, 2024 at 01:01:07PM -0400, Taylor Blau wrote: > > On Fri, Jul 26, 2024 at 02:13:43PM +0200, Patrick Steinhardt wrote: > > > 57 files changed, 251 insertions(+), 73 deletions(-) > > > > I took a careful read through these patches, and found most of them easy > > to review. I was admittedly a little lost with the "fix various leak" > > patches, and having slightly more context in the commit descriptions > > there would have been helpful. > > Yeah, I was a bit torn here on whether to expand the commit messages or > not. I just didn't think it's all that useful to always say "Variable x > is allocated, but never freed" for trivial cases where allocation and > freeing need to happen in the same function, much less so if we repeat > such commits for every single such variable in the same subsystem. So I > just threw those fixes into a single bag. I think sometimes that level of detail is useful, e.g., for tracking where the leak came from, if it's always existed, etc. But I agree enumerating each leak in an otherwise straightforward commit is not a good use of author or reviewer time. I think more useful would be enumerating the kinds of leaks that you clean up. Thanks, Taylor
