Skip to content

Conversation

@ntwb
Copy link
Member

@ntwb ntwb commented Jun 7, 2017

This is a partial revert of 7c9cfe4, originally it was added so that it was in sync with WordPress' .editorconfig, rather WordPress should follow suit of how it's done here.

Follow up to #1029 / #1022

…n` and `*.yml` files.

This is a partial revert of 7c9cfe4, originally it was added so that it was in sync with WordPress' `.editorconfig`, rather WordPress should follow suit of how it's done here.
@ntwb
Copy link
Member Author

ntwb commented Jun 8, 2017

Upstream core ticket https://core.trac.wordpress.org/ticket/40946

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Should we restore tabs to composer.json here as well?

Do .yml files need two-spaces generally, or is it just Travis being picky?

@nylen
Copy link
Member

nylen commented Jun 8, 2017

The composer.json default is 4 spaces (and composer.lock, which still uses 4 spaces in master), but it looks like composer is pretty good about preserving existing indentation.

@nylen
Copy link
Member

nylen commented Jun 8, 2017

http://www.yaml.org/faq.html

Why does YAML forbid tabs?

Tabs have been outlawed since they are treated differently by different editors and tools. And since indentation is so critical to proper interpretation of YAML, this issue is just too tricky to even attempt. Indeed Guido van Rossum of Python has acknowledged that allowing TABs in Python source is a headache for many people and that were he to design Python again, he would forbid them.

🍿 🍿 🍿

@ntwb
Copy link
Member Author

ntwb commented Jun 9, 2017

I added commit 09b849b that switches composer.json to tabs 🤖

@coveralls
Copy link

Coverage Status

Coverage remained the same at 17.816% when pulling 9c12e12 on ntwb:fix/editorconfig-json into c29f11f on WordPress:master.

@ntwb ntwb merged commit 0f578ea into WordPress:master Jul 10, 2017
@ntwb ntwb deleted the fix/editorconfig-json branch July 10, 2017 01:45
@ntwb ntwb mentioned this pull request Sep 1, 2017
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.

4 participants