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

Enforce new ESLint rules for all .js and .js.erb files #575

Merged
merged 10 commits into from
Jan 4, 2024

Conversation

Splines
Copy link
Member

@Splines Splines commented Dec 20, 2023

Enforces the JavaScript style rules introduced in #568. Note that new rules are added/customized in this PR to better reflect the reality of our codebase.

yarn run eslint --fix .

@Splines Splines self-assigned this Dec 20, 2023
@Splines
Copy link
Member Author

Splines commented Jan 3, 2024

As of a8fc972, our codebase is now fully compliant with the ESLint rules we've set up.

@Splines
Copy link
Member Author

Splines commented Jan 3, 2024

This PR is now ready to be reviewed. Note that regarding testing the frontend, I've only performed some quick manual checks, e.g. made sure the login works, I can click on lectures, see a video and edit some attributes as a lecturer, e.g. add a new homework assignment.

Note that I'd love to have else and else if on the same line as the closing }. However, that's currently not possible with ESLint as far as I know. I've opened a feature request for it here. This is probably very easy to add in later on if the feature was implemented since something like this should be available as an autofix.

When reviewing you might want to use the VSCode diff view instead as GitHub does not correctly displays some changes, e.g. when we deleted tabs it shows it as deleted and added line instead of a modification.

@Splines Splines marked this pull request as ready for review January 3, 2024 14:57
Copy link
Collaborator

@fosterfarrell9 fosterfarrell9 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I also played around a lot with things that could be effected by the refactorization of _selectize_turbolinks_fix.js, but everything seems to work nicely.

@Splines Splines merged commit 99a95d1 into mampf-next Jan 4, 2024
5 checks passed
@Splines Splines deleted the lint/eslint-all branch January 4, 2024 11:47
@Splines Splines mentioned this pull request Jan 4, 2024
1 task
Splines added a commit that referenced this pull request Jan 6, 2024
* Add cypress rules to ESLint & ignore some patterns

* Allow usage of tempusDominus global variable

* Ignore JS files with Sprocket syntax

* Further improve rules, e.g. allow common globals

* Ignore sprocket syntax in cable.js

* Autofix all `.js` and `.js.erb` files

Command used:
`yarn run eslint --fix .`

Still 47 problems (27 errors, 20 warnings) after this.

* Fix variables in turbolink fix

* Prepend unused variables with "_"

* Get rid of unused widget variable

* Fix specs comment tab alignment
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