search

JACoders / OpenJK

Community effort to maintain and improve Jedi Academy (SP & MP) + Jedi Outcast (SP only) released by Raven Software
GNU General Public License v2.0
2.04k stars 617 forks source link

[Meta] POC: introduce clang-format #1203

Open Razish opened 9 months ago

Razish commented 9 months ago

DO NOT MERGE

This is a proof of concept for what introducing clang-format would look like.

Highly destructive for forks, but so much nicer to work with. I typically work with format-on-save, but have provided a helper script (scripts/format-code.sh) that could also be repurposed by CI to check if PRs adhere to code formatting rules.

Ref #1202

bartrpe commented 8 months ago

uh, incorporating that into the forks would be a nightmare beyond belief

I'm afraid at this point it kinda has to stay the way it is. I'm afraid OpenJK reached the point in development where doing full reformatting is just not viable anymore ;_;

I tried incorporating it locally onto various popular forks - EternalJK, GalaxyRP, ZykMod... and there's so much mess and problems with mods that ran bigger changes it's a nightmare. Merge conflicts are all over the place. The problems are often barely readable, diff returns spewage at times. At this point, if you had a mod change too much stuff and rework entire large blocks of code in between unchanged blocks and lines, then the whole file needs full, manual review. Every single file, line by line gets returned as spewage.

I tested it by cloning EternalJK/GalaxyRP/others onto my local drive, and then adding OpenJK as remote. Just go at it with this: git remote add clangFormatTest https://github.com/JACoders/OpenJK.git and then simply fetch the remote branches and try tinkering with it. Results are terrible.

Essentially, if mods introduce deeper changes that are not within completely separate blocks of code, everything turns into a mess.

I'm really afraid it's not viable at this point. If it goes into master then people trying to auto-sync to upstream will encounter terrible mess far beyond most people ability to control.

Hell, honestly, while I could make it work I would die inside trying to fit it into GalaxyRP.

Razish commented 8 months ago

I wonder if it would be easier for forks if they pre-formatted with the same clang-format config before merging. Would still require large coordination and likely still be an extremely messy merge, but...probably smaller blast radius? Still not worth it :upside_down_face: