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.

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

Draft - Proposal for code linting #3839

Merged
merged 15 commits into from
Feb 23, 2024

Conversation

1337LutZ
Copy link
Contributor

@1337LutZ 1337LutZ commented Oct 7, 2023

Best to check out this branch and run the commands for yourself.

Didn't commit all the file changes that would happen because of this (over 300).

Have a branch where I ran this command but haven't fixed all the code smells/warnings yet (there's a lot). If there's interest I can go through all the files and fixed them in one fell swoop.

  • Added linting commands
  • Added prettier & stylelint
  • Extended eslint configuration
  • Removed .vscode folder from .gitignore so project settings are picked up when the project is cloned
  • Added codeActionsOnSave command for relevant files
  • Added .vscode/extensions.json to notify developers for suggested extensions when working on this project

commands

  • npm run format will auto fix and format css (scss) and js (ts) files
  • npm run lint will lint all js and css files
  • npm run lint:{js/css} will lint all js or css files
  • npm run lint:{js/css}:fix will fix all js or css files based on linting rules
  • npm run prettier will check all js, json and css files for formatting
  • npm run prettier:fix will fix formatting for all js and css files

@kayla-glick
Copy link
Contributor

Hey @1337LutZ 👋 I know this has been open for a while but I'd love to get better linting and prettier in. Would you have any interest in continuing this PR? It's still in draft so I'm not sure if it's ready or not

@1337LutZ
Copy link
Contributor Author

Hey @kayla-glick, Yea sure can do!
I would update this branch with the latest changes then. However once I run the linting there will be A LOT of changes (basically all the frontend). That's why I would prefer to first introduce the linting changes, and then you can validate this PR by running the lint commands yourself?

@1337LutZ
Copy link
Contributor Author

@kayla-glick I updated it with the latest master builds. Feel free to play around with it. I haven't changed any imports / warnings yet since it'll be over 200+ files.

@kayla-glick
Copy link
Contributor

Thanks for that! I'll pull down this branch and take a look at what the formatting does to make sure the settings are set up how we'd want them too

Dockerfile Outdated Show resolved Hide resolved
@kayla-glick kayla-glick marked this pull request as ready for review February 23, 2024 19:26
@kayla-glick kayla-glick merged commit aa19e1c into wowsims:master Feb 23, 2024
1 check passed
@1337LutZ
Copy link
Contributor Author

1337LutZ commented Feb 23, 2024

Sweet, nice to hear!
Just prepare for the 200+ files of changes when you run the lint fix command 👌

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