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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to