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

Tabs to Spaces #558

Closed
damassi opened this issue Nov 23, 2015 · 4 comments
Closed

Tabs to Spaces #558

damassi opened this issue Nov 23, 2015 · 4 comments

Comments

@damassi
Copy link

damassi commented Nov 23, 2015

Browsing the code, i'm noticing that you're using tabs instead of spaces, which blows out code viewing unnecessarily into 8 spaces for every tab:

screen shot 2015-11-23 at 11 53 30 am

Idiomatic JS is typically 2 soft spaces, which significantly cuts down on read time across a codebase, while also making contributions easier since people can read the code at a glance.

@aduth
Copy link
Contributor

aduth commented Nov 23, 2015

You can add a ?ts=4 (?ts=2, etc.) parameter to reduce the spacing while previewing on GitHub. GitHub has begun to incorporate some .editorconfig features, so we could choose to assign a tab_width parameter in our configuration, though this would likely take precedence over a developer's editor settings if an EditorConfig plugin is in use.

blowery added a commit that referenced this issue Nov 23, 2015
This is more like most JS out there and changes how GitHub displays code. See isaacs/github#170 (comment)

Refs #558
@rralian
Copy link
Contributor

rralian commented Nov 23, 2015

It's a bit of a WordPress legacy to prefer tabs over spaces. https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#spacing

And that's been picked up in this project (everyone came from WordPress after all). https://github.com/Automattic/wp-calypso/blob/master/docs/coding-guidelines/javascript.md#spacing

I agree though, seems like the consensus in the javascript community is to prefer two-spaces for indentation instead of tabs. I think it would be a good idea for us to align with that (even if I always did prefer tabs).

@damassi
Copy link
Author

damassi commented Nov 23, 2015

As a previous 4-spacer, 2 really grows you after a bit of time. But yeah, since you're moving over to JS on such a large scale using React and so on, it would be best to align sooner rather than later : ) Aside from that, great work.

@nb
Copy link
Member

nb commented Nov 24, 2015

Thanks, @damassi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants