aaron.ballman added a comment.

In D148835#4286905 <https://reviews.llvm.org/D148835#4286905>, @erichkeane 
wrote:

> In D148835#4284871 <https://reviews.llvm.org/D148835#4284871>, @cjdb wrote:
>
>> I've had some very good input about why this probably shouldn't go ahead: 
>> git history erasure :')
>
> While that is a common criticism, if we're going to do this at all, we should 
> do it in a single patch, as we can use --ignore-rev: 
> https://akrabat.com/ignoring-revisions-with-git-blame/
>
> This is a pretty common thing to do, and I don't think it should prevent us 
> from doing this, which I think is the 'greater good' here.  The alternative 
> is to continue having a bunch of unrelated patches piece-meal 'erase' git 
> history as people accidentally fix these.

We already ignore blame revisions like this today: 
https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs

However, I'd ask that we hold off on this change unless we have some tooling in 
place that rejects new additions of trailing whitespace, otherwise we're going 
to end up in this exact same situation again in another week or two. When CI 
starts failing on patches that insert *new* trailing whitespace, then I'm all 
for this change because we can fix it once and hopefully keep it fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

Reply via email to