https://github.com/JDevlieghere commented:
I haven't had a chance to look at this until now. I would like if you could split this further into two patches: one that changes the existing interfaces and one which adds (potentially unused) new functions. As far as I can tell, the `HasTargetRunSinceMe` is a completely new concept/feature and so that doesn't belong in the same patch that's adding `const` and changing the behavior of `ReadWatchpoint`. The goal of LLVM's reliance on NFC patches is to eliminate everything that's unrelated to the new code, to make reviewing easier by being more focused. I keep find myself having to go back and forth to figure out if something is existing logic changing or new (unused) stuff being added. As a reviewer I want to: 1. Review the NFC patch and verify nothing is changing/breaking. 2. Review the functional change, and verify it does the right thing and is properly tested. 3. Repeat 1 & 2 until the feature's done. https://github.com/llvm/llvm-project/pull/163695 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
