[libcxx-commits] [PATCH] D140550: [String] Refactor unit tests to allow easy addition of new allocators

Tue Jan 10 01:49:01 PST 2023

bemeryesr added a comment.

In D140550#4037134 <https://reviews.llvm.org/D140550#4037134>, @ldionne wrote:

> Thanks for the patch. It took some time to look at it, so it must have been pretty tedious to write it, but I think it's a good improvement overall.
>
> Given the huge number of changes, I might have made the changes without touching the formatting at all to reduce the diff as much as possible, and then do the formatting changes (or not at all). But anyway, they're here now and I've reviewed it like that, so let's not change it. Just something to consider for the future.
>
> Let's try to get this landed quickly to avoid getting stuck in endless rebase conflict resolutions.

Thanks for the quick review! I will work on this today.

I completely agree with you about the formatting changes. I actually initially tried to have this PR just include the actual changes (and not the formatting changes) but when I submitted the PR using arc, it forced me to apply the clang formatting (or at least it seemed that way to me, perhaps I did something wrong). Is there a way to submit the patch without applying the clang formatting to all touched files?

As a side note, based on your comments about the formatting, perhaps you want to weigh in on this PR of mine: https://reviews.llvm.org/D140612.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140550