sbc100 added a comment. In D148835#4288151 <https://reviews.llvm.org/D148835#4288151>, @erichkeane wrote:
> In D148835#4288148 <https://reviews.llvm.org/D148835#4288148>, @cjdb wrote: > >> In D148835#4286924 <https://reviews.llvm.org/D148835#4286924>, >> @aaron.ballman wrote: >> >>> 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 >> >> Oh cool, I had no idea this was possible :) >> >>> 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. >> >> Since Clang doesn't have pre-commit CI, how can we meaningfully progress >> here? > > Phab at least has Pre-commit CI, i would expect that to be sufficient? It > would at least cut down accidential whitespace by orders of magnitude. Yes, isn't this already covered by the existing use of `clang-format` on upload to phabricator (`arc diff`) ? At least for me it always gives me warnings if I try to upload anything that doesn't match clang-format. 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
