-
-
Notifications
You must be signed in to change notification settings - Fork 234
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
chore: keep the indentation style homogeneous accross IDEs #649
Conversation
We have a special ts lint ruleset, so if you have tslint enabled it should keep things the same, right? Editor config is good, are we added it as a backup for those contributors who are missing tslint ? |
I have tslint enabled, and to my best knowledge, none of my files have been staged with an offending indentation. However the husky hook only happens at pre-commit time (behind the back of the developer). This file (along with the plugin, for the IDEs that don't natively support it) would tidy the file when it's being saved, under the eyes of the committer. |
@@ -0,0 +1,8 @@ | |||
root = true | |||
|
|||
[*] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to be using 4 spaces for certain JSON files, so perhaps let's account for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to be using 4 spaces for certain JSON files, so perhaps let's account for that.
@P0lip I can think of three options:
- Include in the PR normalized versions (to two spaces wide indentation) of those files (under
/src/meta
, for instance) - If those indentations have to be preserved (to ease diffing comparison with an external source of truth for instance), add another
.editorconfig
file under this hierarchy specifying that.json
files here and below are expected to be indented with 4 spaces - Update the root
.editorconfig
file to replace the*
with something more restrictive which would explicitly not consider json (eg.[*.{ts,js,yaml}]
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go with [*.{ts,js,yaml}]
so we can avoid doing anything funky with Markdown files (like trimming intentional trailing whitespace).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.
That said, we can still have charset set to utf-8 for all file types.
Re YAML - I'd stick to indentation only, as trailing spaces and newlines might be intentional in some cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philsturgeon @P0lip Latest proposal should encompass all those requirements above
Helps maintaining the code style across IDEs cf. https://editorconfig.org/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Let's wait for Phil's take on the latest changes and we can merge it.
I'll go ahead and merge it. If Phil has any objections, we can address them in the next PR. |
Helps maintaining the code style across IDEs
cf. https://editorconfig.org/
Checklist
Does this PR introduce a breaking change?
While working on #635 I noticed that my IDE (VSCode) insisted on trying to format files with four spaces.
This file combined with the VSCode extension (https://editorconfig.org/#download) helps keeping the code style tamed.