Skip to content

Conversation

@wmcmurray
Copy link
Contributor

In case you didn't know 🤷‍♂️ , this will automatically set almost any text editor to the proper config :
utf-8, 2 tabs, LF newlines and no trailing white spaces (from Mr.doob's Code Style)

It makes things much more easier ! And different config can be applied to different files types, etc.

You just need the plugin for your text editor : https://editorconfig.org/ (there is one for Atom :atom: )

If this gets merged, I guess it could be mentioned in the wiki page before last point.

@mrdoob
Copy link
Owner

mrdoob commented Mar 11, 2019

Hmm, is very rare these days to see Pull Requests that don't follow that code style.
Makes me wonder if this is really needed...?

@gkjohnson
Copy link
Collaborator

@mrdoob FWIW I have an uncommitted .editorconfig and .eslintrc.json file (that uses the eslint-config-mdcs config) in my local three.js repo to ensure that code fits the expected style. I think maintaining these files along side the main repo will make it easier to track if the code style ever changes in the future (switching from var to let / const for example).

I would vote for adding this and an eslint file to the repo.

[*.{js,ts,html}]
charset = utf-8
indent_style = tab
indent_size = 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is merged I don't think the config should enforce indent width considering it is only a visual configuration and doesn't affect the file written to disk. I often use 4 columns to indent, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, don't hesitate to tweak it to your needs, I just wanted to start the debate..
You guys know this codebase much more than I do ! 🙇‍♂️

@wmcmurray
Copy link
Contributor Author

@mrdoob well it's not "needed", but it puts a burden aside for every person wanting to contribute 🤷‍♂️

...and it's pretty common : vuejs, react, angular, laravel, php, nodejs , name it !

@mrdoob
Copy link
Owner

mrdoob commented Mar 15, 2019

I see I see. Okay!

@mrdoob mrdoob added this to the r103 milestone Mar 15, 2019
@mrdoob mrdoob merged commit 689b187 into mrdoob:dev Mar 15, 2019
@mrdoob
Copy link
Owner

mrdoob commented Mar 15, 2019

Thanks!

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.

3 participants