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

Enable eslint indent rule #5666

Merged
merged 3 commits into from
Feb 19, 2025
Merged

Conversation

vincentfretin
Copy link
Contributor

Description:

I added a prettier config in #5665 that applies 2 spaces indent only for html and js inside the html for new contributed examples.
For js files that 2 spaces indent wasn't enforced.

Changes proposed:

  • Enable the eslint indent rule to use 2 spaces in js files, use proper options to minimize code changes
  • Fix all indent errors

uvs.push(uc, vc,
uc + ul * cosa, vc + vl * sina,
uc + ul * cosb, vc + vl * sinb);
uvs.push(uc, vc, uc + ul * cosa, vc + vl * sina, uc + ul * cosb, vc + vl * sinb);
Copy link
Member

Choose a reason for hiding this comment

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

This rule makes the code less legible. It's good to be able to break a complicated list of parameters passed to a function in multiple lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the same line in one other place in this file, so I decided to do the same for this one, but I can do the other way around, put it in several lines at both places.
The indent rule with "CallExpression": { "arguments": "first" } allowing to align the other lines on the same position as the first argument just warned about 1 missing space indentation for this particular case.
I fixed it in the next commit, you're right that's more readable.

@dmarcos
Copy link
Member

dmarcos commented Feb 19, 2025

Thanks!

@dmarcos dmarcos merged commit 1fe1247 into aframevr:master Feb 19, 2025
3 checks passed
@vincentfretin vincentfretin deleted the eslint-ident branch February 20, 2025 08:09
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