search

flybywiresim / installer

FlyByWire Simulations installer
GNU General Public License v3.0
92 stars 82 forks source link

chore: lint overhaul #458

Closed Benjozork closed 3 months ago

Benjozork commented 4 months ago

Summary of Changes

FoxtrotSierra6829 commented 4 months ago

Still unsure how to feel about the change from 4-space to 2-space indentation. Are we changing this on all FBW repositories? I saw you did in flybywiresim/simbridge. Maybe something to discuss with other devs?

frankkopp commented 4 months ago

Still unsure how to feel about the change from 4-space to 2-space indentation. Are we changing this on all FBW repositories? I saw you did in flybywiresim/simbridge. Maybe something to discuss with other devs?

Yes - all repos should have the same linting.

Also 2 spaces are imho good - the C++ code already has this for example. I would vote for 2 vs. 4.

FoxtrotSierra6829 commented 4 months ago

Still unsure how to feel about the change from 4-space to 2-space indentation. Are we changing this on all FBW repositories? I saw you did in flybywiresim/simbridge. Maybe something to discuss with other devs?

Yes - all repos should have the same linting.

Also 2 spaces are imho good - the C++ code already has this for example. I would vote for 2 vs. 4.

I find more 4 spaces easier to read over multiple nested blocks. The main advantage to me would be that the deeper blocks have less of an issue to exceed the maximum line length.

Benjozork commented 4 months ago

Maybe that will discourage excessive nesting then 😄

https://www.youtube.com/watch?v=CFRhGnuXG-4

tracernz commented 4 months ago

4 is definitely a lot more readable, with the side benefit of discouraging excessive nesting. If only we could all use tabs thus could set our own preference in our editor of choice.

derfelix54 commented 3 months ago

Hey @Benjozork , after those changes I cannot build the master branch locally with npm run package anymore. Is there something I have to do, not mentioned in the readme?

derfelix54 commented 3 months ago

Hey @Benjozork , after those changes I cannot build the master branch locally with npm run package anymore. Is there something I have to do, not mentioned in the readme?

Update: source-map is missing in the package.json. After npm install it built.