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

Extend eslint-rules #986

Merged
merged 2 commits into from
Dec 23, 2022
Merged

Extend eslint-rules #986

merged 2 commits into from
Dec 23, 2022

Conversation

narickmann
Copy link
Contributor

Added more eslint-rules like described here.
This resolves #985

Haven't added "array-bracket-spacing": ["error", "always"] yet. Every useState declaration will look like this: const [ state, setState ] = useState() not quite sure, if I like how it looks. I think I will wait for feedback for this rule.

Added more eslint-rules like described in [this issue](elan-ev#985).
This resolves elan-ev#985

Haven't added `"array-bracket-spacing": ["error", "always"]` yet. Every useState declaration will look like this: `const [ state, setState ] = useState()` not quite sure, if I like how it looks.
I think I will wait for feedback for this rule.
@narickmann
Copy link
Contributor Author

Should I add comments to each rule that briefly explain what the rules are for? (like in the linked issue)

One line was over max-length (120) and missed space between brackets.
@lkiesow
Copy link
Contributor

lkiesow commented Dec 23, 2022

Should I add comments to each rule that briefly explain what the rules are for? (like in the linked issue)

Since ESLint has a good documentation, and it's really easy to look up the rules (if you don't already understand them from their name alone), I don't think it is necessary.

@lkiesow lkiesow merged commit 1227764 into elan-ev:master Dec 23, 2022
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Very nice! That makes stuff way easier.

I realize this is already merged, but I thought my comments might be useful anyway.

.eslintrc.json Show resolved Hide resolved
"eol-last": ["error", "always"],
"func-call-spacing": ["error", "never"],
"indent": ["error", 2, { "SwitchCase": 1, "MemberExpression": "off" }],
"jsx-quotes": ["error", "prefer-single"],
Copy link
Member

Choose a reason for hiding this comment

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

I just checked and Studio used more double quotes before. Enabling this rule causes 77 errors. Enabling prefer-double only causes 23 errors. Tobira uses double quotes; the editor seems to use double quotes as well. It's also closer to HTML (which JSX wants to mimic). So I'd have preferred to use double quotes.
Should we still change this or just keep it as is now?

@@ -136,7 +136,7 @@ export class SettingsManager {
// the value at the end.
let obj = rawUrlSettings;
const segments = key.split('.');
segments.slice(0, -1).forEach((segment) => {
segments.slice(0, -1).forEach( segment => {
Copy link
Member

Choose a reason for hiding this comment

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

The space left of segment doesn't belong here, right?

}
}}
}} // eslint-disable-line
Copy link
Member

Choose a reason for hiding this comment

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

Hu, why is that needed?

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.

Extend eslint-rules
3 participants