Skip to content

[Tech] Disable eslint during development, added options to editorconfig #1132

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.

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

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

dawidgarus
Copy link
Contributor

Added addition options to .editorconfig
Disable eslint validation during development

Rationale:
The reason to disable it is that it's infuriating to develop anything with eslint enabled.
You comment single line for then you get bombarded with unused variables and import which must also be commented which will cause new errors.
Moving code around is impossible without getting perfect indents.
I don't see any value with checking that during development, especially since prettier will take care of it anyway.
They'll still be checked during build and pipeline execution.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima
Copy link
Member

I have to say that I don't agree with this one.
Although might be boring, when developing with so many different people that come and go all the time, it's important to enforce at least some basic rules.
I don't know if your editor has this feature but at least on VSCode I set to format (using the current eslint rules) on save, so if I have some ident issue or something like this, when I save the file it gets fixed automatically.

Before adding those rules the code started to become a mess in terms of formatting. At the time we added a lot of rules, some of them were bad for productivity then I removed several, and today we have only the basic set of rules, the recommended ones.
so I believe we need to pass this PR.

@dawidgarus
Copy link
Contributor Author

dawidgarus commented Mar 18, 2022

@flavioislima
It's not about formatting, that's not important and eslint don't force any formatting rules that are taken care by prettier.
The most important thing are rules regarding unused things, as I said:

You comment single line for then you get bombarded with unused variables and import which must also be commented which will cause new errors.

It's only for development. Build command will enforce eslint rules, gitlab pipeline will enforce eslint rules and git hooks will also enforce eslint rules. In other words, it's still impossible to merge into this repo code that doesn't pass eslint rule.

@flavioislima
Copy link
Member

@dawidgarus yes, makes sense then. I though you wanted to disabled eslint at all 😅

@flavioislima flavioislima merged commit 2c32e79 into Heroic-Games-Launcher:main Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants