Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: lint overhaul #458

Merged
merged 18 commits into from
Mar 26, 2024
Merged

chore: lint overhaul #458

merged 18 commits into from
Mar 26, 2024

Conversation

Benjozork
Copy link
Member

Summary of Changes

  • Moves lint to a simpler (and stricter) setup with eslint:recommended, plugin:prettier/recommended and plugin:@typescript-eslint/recommended
  • Fixes lint failures introduced by the above

Benjozork added 16 commits January 6, 2024 08:05
@Benjozork
refactor: remove antd and antd icons
34c5e3f
@Benjozork
fix: further styling fixes
8199c0b
@Benjozork
fix: further styling fixes
a4ff01f
@Benjozork
fix: further styling fixes
472a3fb
@Benjozork
fix: broken changelog
7d4caad
@Benjozork
fix: further styling fixes
4121d77
@Benjozork
fix: lint
d872241
@Benjozork
fix: remove testing code
c5b04ec
@Benjozork
refactor: move lint to prettier
35e8edf
@Benjozork
style: run lint
6b5af2c
@Benjozork
refactor: add react-hooks and tailwind lint
e70fe8b
@Benjozork
style: run lint
0bd1946
@Benjozork
refactor: fix lint issues
30ab1f0
@Benjozork
Merge branch 'master' into refactor/lint
4e22ec4 
# Conflicts:
#	.github/CHANGELOG.yaml
#	package-lock.json
#	src/renderer/components/AddonSection/Configure/TrackSelector.tsx
#	src/renderer/components/AddonSection/ReleaseNotes/index.tsx
#	src/renderer/components/App/index.tsx
#	src/renderer/components/ErrorModal/index.tsx
#	src/renderer/components/InstallerUpdate/index.tsx
#	src/renderer/components/LocalApiConfigEditUI/index.tsx
#	src/renderer/components/Modal/ErrorDialog.tsx
#	src/renderer/components/Modal/index.tsx
#	src/renderer/components/SentrySessionCard.tsx
#	src/renderer/components/SettingsSection/Developer.tsx
#	src/renderer/components/SettingsSection/Download.tsx
#	src/renderer/components/SettingsSection/General.tsx
#	src/renderer/components/Toggle.tsx
#	src/renderer/components/WindowActionButtons/index.tsx
#	src/renderer/fbw.scss
#	src/renderer/index.tsx
@Benjozork
style: run lint
3bd1bb4
@Benjozork
chore: fix lint
d62d962
@Benjozork Benjozork changed the title chore: update lint chore: lint overhaul Feb 25, 2024
frankkopp
frankkopp approved these changes Feb 27, 2024
View reviewed changes
Copy link
Member

@frankkopp frankkopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking line by line is of course impossible ;)
But by spot checking changes it seems this worked well.
Also no issues in the Installer itself.

@FoxtrotSierra6829
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

Maybe that will discourage excessive nesting then 😄

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

@tracernz
Copy link
Member

tracernz commented Mar 7, 2024

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.

@Benjozork Benjozork merged commit 0458df8 into master Mar 26, 2024
2 checks passed
@derfelix54
Copy link

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
Copy link

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.

Benjozork added a commit that referenced this pull request Aug 21, 2024
@Benjozork
chore: lint overhaul (#458)
e4b8923 
* refactor: remove antd and antd icons

* fix: further styling fixes

* fix: further styling fixes

* fix: further styling fixes

* fix: broken changelog

* fix: further styling fixes

* fix: lint

* fix: remove testing code

* refactor: move lint to prettier

* style: run lint

* refactor: add react-hooks and tailwind lint

* style: run lint

* refactor: fix lint issues

* style: run lint

* chore: fix lint

* fix: simplebar import

* fix: selected track styling

(cherry picked from commit 0458df8)
@JoaoOliveira85 JoaoOliveira85 mentioned this pull request Oct 23, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoxtrotSierra6829 FoxtrotSierra6829 FoxtrotSierra6829 left review comments

@frankkopp frankkopp frankkopp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Benjozork @FoxtrotSierra6829 @frankkopp @tracernz @derfelix54