search

flybywiresim / aircraft

The A32NX & A380X Project are community driven open source projects to create free Airbus aircraft in Microsoft Flight Simulator that are as close to reality as possible.
https://flybywiresim.com
GNU General Public License v3.0
5.14k stars 1.12k forks source link

Linting and code formatting #212

Closed disposedtrolley closed 4 years ago

disposedtrolley commented 4 years ago

I've witnessed a tremendous amount of progress being made on this project in the few days I've known about it. It's incredible to see how passionate the flight sim community is in giving up their time to work on freeware!

In an effort to keep this project scalable and maintainable, what do we think about introducing linting and code formatting for the HTML, CSS, and JS files in particular? This would allow for a consistent style across the codebase, reduce the amount of complexity required when dealing with any merge conflicts, and reduce overhead for contributors when thinking about how code should be formatted. The linter can also catch basic issues via static analysis.

I'm happy to investigate some tools and spike it out in a PR if there's interest. ESLint is quite comprehensive and does double duty of linting and code formatting.

The main challenge will be to roll this out with minimal disruption to inflight features and PRs. Perhaps we can introduce another step in CI to run the formatter and commit changes?

Benjozork commented 4 years ago

I've been thinking about this too.

EditorConfig would also be useful to guarantee end-of-line and indentation consistency.

michaeldiguiseppi commented 4 years ago

We could also implement prettier for code formatting.

Benjozork commented 4 years ago

@michaeldiguiseppi, in my opinion Prettier alone is too opiniated and enforces some annoying rules sometimes. I also think it's not very suitable for us since a lot of Asobo files are already weirdly formatted and Its lack of configurability would not let us tune those warnings down.

ESLint can inherit Prettier's rule set anyway and has the advantage of being able to let the user disable certain rules.

Curtis-VL commented 4 years ago

Ironically, a lot of the Asobo code follows different formatting rules too, which doesn't make things any easier. :)

Benjozork commented 4 years ago

Ironically, a lot of the Asobo code follows different formatting rules too, which doesn't make things any easier. :)

Yeah, that's the thing.

I think we can start by adding linter config files (ESLint, EditorConfig, not sure what to use for XML) and encouraging people to use them. Let them print out warnings in checks. Encourage people to fix formatting whenever they have the chance.

Then, when a lot of code has better formatting, we can start enforcing it.

adenflorian commented 4 years ago

i just added eslint in #1130, will figure out prettier or something for all other files

adenflorian commented 4 years ago

closing in favor of #761