[llvm] [TableGen] [code-cleanup] Refine TableGen code to comply with `clang-tidy` checks and remove unused imports (PR #113318)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 22 08:17:33 PDT 2024


jurahul wrote:

IMO, discouraging such changes to preserve git history is not a great
argument. We have tools like git blame which help recover the history. Yes
you may have to make a few more hops in the git blame chain. Also packaging
NFC and functional changes together is also not recommended AFAICT.

On Tue, Oct 22, 2024 at 8:13 AM Krzysztof Parzyszek <
***@***.***> wrote:

> What’s the best way to do this? Split it per file?
>
> The issue is that "formatting" commits introduce noise in the file
> history, which can affect things like git blame. There are ways around
> it, for example, blame allows you to give a list of commits to ignore (we
> have it in .git-blame-ignore-revs), but you still have to manually
> provide that option. Some tools may not be doing it, and so they can trace
> some change back to an NFC commit.
>
> The "preferred" approach it to tack it on top of some functional commit,
> i.e. make such changes to the code that the commit was changing anyway. The
> downside is that this process may never finish all the intended changes.
>
> Paul is the code owner of TableGen, so I defer to him to make the final
> decision. This change is fine if there is a consensus to do it, and I'm not
> opposed.
>
>> Reply to this email directly, view it on GitHub
> <https://github.com/llvm/llvm-project/pull/113318#issuecomment-2429561520>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/APRMUB3YOU2VND6USFSCWRLZ4ZTSLAVCNFSM6AAAAABQMSTYAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRZGU3DCNJSGA>
> .
> You are receiving this because your review was requested.Message ID:
> ***@***.***>
>


https://github.com/llvm/llvm-project/pull/113318


More information about the llvm-commits mailing list