clang-format isn't being properly enforced

[barnes88]

I don't think the method setup in #236 is properly enforcing clang-format, because the dev branch has already diverged quite a bit from the formatting (I ran it on the latest dev commit and there are quite a few changes https://github.com/barnes88/gpgpu-sim_distribution/tree/clang-format-6.4.24). This is annoying if you have your editor set to enforce formatting on save, because it causes a bunch of lines to be changed that you didn't edit.

Maybe we can have Jenkins run a formatting stage diff so we can reject PRs that aren't properly formatted. Or we need everyone in the lab to install the pre-commit hooks.

@FJShen

[tgrogers]

Yeah we used to have a format checker. Please flag junrui to resurrect it. We have move to GitHub ci and away from maintaining our own Jenkins server.

…

On Tue, Jun 4, 2024 at 14:26 Aaron Barnes ***@***.***> wrote: I don't think the method setup in #236 <#236> is properly enforcing clang-format, because the dev branch has already diverged quite a bit from the formatting (I ran it on the latest dev commit and there are quite a few changes https://github.com/barnes88/gpgpu-sim_distribution/tree/clang-format-6.4.24). This is annoying if you have your editor set to enforce formatting on save, because it causes a bunch of lines to be changed that you didn't edit. Maybe we can have Jenkins run a formatting stage diff so we can reject PRs that aren't properly formatted. Or we need everyone in the lab to install the pre-commit hooks. @FJShen <https://github.com/FJShen> — Reply to this email directly, view it on GitHub <#305>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7UY4NLUNNOGKXO4B4WRSDZFYBFLAVCNFSM6AAAAABIZBLXCKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMZTIMJSG42TGOI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[JRPan]

Or I can spawn a job to run the format script before merge.

[FJShen]

Sorry for your inconvenience. Accel-Sim PR #236 (and GPGPU-Sim PR 60, which is still being reviewed) was an one-shot commit that re-formatted the code. The configuration file (.pre-commit-config.yaml) for "pre-commit" is included in the PR, but so far we don't have anything automated.

[FJShen]

@JRPan do you have anything implemented for the automatic formatting?

[JRPan]

Should we do automatic formatting before merging? Or reject non-formatted PR and ask people to format it?

[barnes88]

I think having automatic formatting would be easiest, that way people don't have to worry about trying to match the version of clang-format