[PATCH] D54402: Extract method to allow re-use

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D54402#1295420, @steveire wrote:

> I think this commit is fine without tests.


That's not something we do except under specific extenuating circumstances (NFC fixes or changes for which existing coverage suffices), per our developer policy.

> The new method remains internal to the file but follow-up commits add public interface and tests.
> 
> I'll push the commits separately. I'll not be squashing them.

I'd prefer to squash them at least into commits that have tests for the functionality. More specifically: if the functionality is testable, it needs tests when it's committed. If it's not testable, squash it into the most closely-related commit that is testable. In terms of review cycles, I'm fine with "this functionality will be commit with that functionality, whose tests cover this.", though I'd personally prefer future reviews to come pre-squashed to cut down on confusion.


Repository:
  rC Clang

https://reviews.llvm.org/D54402