[libcxx-commits] [PATCH] D144734: [libcxx] Enable support for static and debug Windows runtimes

[Andrew Ng via Phabricator via libcxx-commits]

andrewng added a comment.

>>> I like that people try to be CI "friendly". But I consider a clean git history more important. When a patch breaks something on a platform we revert the entire patch, so when 3/4 of the changes are an improvement 4/4 will be reverted.
>>
>> Yes, a clean git history is important but so is getting better platform test coverage.
>
> To me, this isn't even so much about clean git history, it's primarily for reasoning about and deciding about each detail separately.
>
> Right now, looking at the patch as a whole, my verdict for each of the 3-5 changes is "maybe". But it's very hard to move from "maybe" to "yes" unless the detail is crystal clear and all implications are discussed/brought up/agreed on. Split up, it's much easier to move a few of them from "maybe" to "yes", and discuss/settle/improve the details of the remaining bits.

Yes, I agree it will make review/evaluation much easier. It's been quite tricky finding all the issues and authoring this patch and this is the "cleaned up" version of my original patch which was even more complex!

> Also FWIW, I might be able to help out with this (splitting/testing/adjusting of these patches) in a few weeks (sometime in July maybe).

This is much appreciated and I may well have to take you up on the offer as I don't know how much time I'll have to spend on it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144734/new/

https://reviews.llvm.org/D144734