Open-Assistant
Open-Assistant copied to clipboard
LAION-AI
Add eslint rules for consistent imports
To have consistent imports, add eslint rules to encourage (and perhaps enforce?) a preferred ordering.
The rules in question:
https://eslint.org/docs/latest/rules/sort-imports
Stick with the default eslint sorting rules (link), in case we are not happy with it, we can change it later.
Notes: Should we reformat all files at once?
I'm quite new to all of this though I thought that maybe we could have a script that fix the import that can be run before there is a commit. Also I was having a problem installing sort-import so I tried to do it with isort as an alternative
This is what I have done https://github.com/Jac-Zac/Open-Assistant/tree/sort-imports, could it be good any other suggestions ?
@Jac-Zac thanks! we welcome all contributions.
For the backend, we already have pre-commit hooks that should guarantee formatting with isort.
However, this issue covers the typescript code for the website, using eslint. Would you like to contribute?
This is what I have done https://github.com/Jac-Zac/Open-Assistant/tree/sort-imports, could it be good any other suggestions ?
thanks for the suggestion. As @AbdBarho says, we already have enforcement of consistent imports in python, due to pre-commit. This issue is about the typescript code. We are already running next lint also in pre-commit, which includes (as far as I can tell) eslint, but what's left to do is to define the eslint-rules, because so far we're just using default ones.
Oky I'll try but as I said I'm not sure I can help if I can't do it in a few hours I'll let you now else I'll try to write it
I hope this is more on the line on what you were asking for https://github.com/Jac-Zac/Open-Assistant/tree/new-eslintrc.cjs
I have created a config file .eslintrc.cjs and with the default configuration required by following what you gave here and also this. (I lost a bit of time by putting it in the root directory before I realize it should have been in the website directory)
@Jac-Zac great! could you please replace or extend this file? https://github.com/LAION-AI/Open-Assistant/blob/main/website/.eslintrc.json
I keep on getting the error "ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct.". But I get it also if i run the old eslintrc.json thus I suppose that maybe I'm missing something to install. If someone knows way I'm running into the problem or if it is just a problem on my head it would be helpful. However I have replaced the old .eslintrc, with this .eslintrc.json. Does that work for you (sorry If I'm not that helpful or I'm wasting time).
@Jac-Zac no worries! could you please create a merge request? I can check whats the problem.
Also I installled this npm install --save-dev @typescript-eslint/parser @typescript-eslint/eslint-plugin eslint typescript
Thous typescript might need to be installed to make the pre-commit hooks work
Thous typescript might need to be installed to make the pre-commit hooks work
the pre-commit hook runs bash -c 'cd website && npm install && npm run lint' (in .pre-commit-config.yaml), so anything in package.json will be there