This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [test] Remove clang-format comments from generated files. NFC.
AbandonedPublic

Authored by ldionne on Jan 11 2022, 7:19 AM.

Details

Reviewers
Quuxplusone
Group Reviewers
Restricted Project
Summary

These files are auto-generated and will never be clang-formatted, so the comments are just noise.
Meanwhile they introduce a moral hazard, that new contributors see all these clang-format comments in the repo and think they set a precedent for clang-format comments in *non*-generated parts of the codebase.

(I propose also two followups: one to remove clang-format comments from the human-written parts of the codebase, and one to disable the check-format step in libcxx/utils/ci/run-buildbot — it seems to be causing vastly more harm than benefit.)

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Jan 11 2022, 7:19 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJan 11 2022, 7:19 AM

Before we start doing this I prefer to first discuss what we want to do with clang-format. I really like to use it, even when it's not always perfect it's more consistent than human formatted code.

I really dislike removing the 3 clang-format off markers from human edited code. I added them for a good reason.

Before we start doing this I prefer to first discuss what we want to do with clang-format. I really like to use it, even when it's not always perfect it's more consistent than human formatted code.

I strongly disagree that clang-format is "more consistent"; generally what we find is that taking clang-format's advice produces inconsistencies within the codebase. However, I suggest that at some point we should all get together and have a meeting about this issue, because it continues to be a source of friction over and over with new contributors.

I really dislike removing the 3 clang-format off markers from human edited code. I added them for a good reason.

Can you clarify what you mean here? There's no human-edited code in this PR, except for the Python generators themselves. All the .cpp files touched in this PR are maintained via the Python scripts.

Before we start doing this I prefer to first discuss what we want to do with clang-format. I really like to use it, even when it's not always perfect it's more consistent than human formatted code.

I strongly disagree that clang-format is "more consistent"; generally what we find is that taking clang-format's advice produces inconsistencies within the codebase. However, I suggest that at some point we should all get together and have a meeting about this issue, because it continues to be a source of friction over and over with new contributors.

I agree. I prefer to have that discussion before removing removing clang-format comments from the code base.

I really dislike removing the 3 clang-format off markers from human edited code. I added them for a good reason.

Can you clarify what you mean here? There's no human-edited code in this PR, except for the Python generators themselves. All the .cpp files touched in this PR are maintained via the Python scripts.

That's regarding "one to remove clang-format comments from the human-written parts of the codebase" in the description of this review.

Let's have a discussion on Discord. I agree we need to settle on this question. It is causing friction for new and existing contributors, and it keeps coming up.

ldionne commandeered this revision.Jul 12 2023, 5:23 AM
ldionne edited reviewers, added: Quuxplusone; removed: ldionne.

I think this is safe to abandon now.

Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 5:23 AM
ldionne abandoned this revision.Jul 12 2023, 5:23 AM