DavidSpickett wrote:

With a reasoning this would have been fine to merge without review. As it is, 
this should not have been merged.

I suspect the reasoning is "so I can compare across class A, B and C which have 
10s of members scattered around". Which I would have agreed with, but we cannot 
go on suspicions during review.

With this sort of async, remote, overlapping time zone sort of work, lean to 
the side of stating the obvious. Not everyone is reading everything you do and 
they'll have forgotten most of it if they did. Also consider that reviewers 
have minutes per PR, and they might not get back to it until 24 hours later. So 
anything you can do to save another round of back and forth is incredibly 
valuable to everyone involved.

And if people think your're too verbose (something I have been told a few times 
:) ), then you can adjust, no problem.

https://github.com/llvm/llvm-project/pull/183414
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to