dojo / dojo-package-template

Dojo 2 - template to clone for creating packages (internal use)
Other
6 stars 18 forks source link

Use spaces, not tabs, in package.json? #29

Closed novemberborn closed 8 years ago

novemberborn commented 8 years ago

Certain npm commands, like npm install --save, will update the package.json. npm will reindent the file using spaces, not tabs.

Might be good to just use spaces in the package.json file and avoid having to deal with the whitespace churn. Same may apply for the other JSON files.

novemberborn commented 8 years ago

It also seems to change the file mode:

diff --git a/package.json b/package.json
index 27fac06..80f0a66
--- a/package.json
+++ b/package.json
old mode 100755
new mode 100644
kitsonk commented 8 years ago

Traditionally we have been morally opposed to npm's decision to not respect the source formatting and have continued to self-flagellate in order to keep the tabs in our package.json in line with the white spacing in the rest of the project, which we have enforced via our linting and tooling (for example, we explicitly use tabs when rewriting tsconfig.json.

That does mean we never use npm install --save in the packages we manage and add our dependencies directly.

Personally, I am disappointed that npm has taken such a narrow view of the world, but it seems like a fugacious argument to battle against it, so it doesn't matter to me, though I don't know if others feel different.

As for 755 versus 644 that is odd why the executable flag was ever turned on for package.json.

novemberborn commented 8 years ago

@kitsonk you almost sound Shakespearean!

This issue links to a bunch of others, and no, I don't think npm is ever going to change its stance on this.

That does mean we never use npm install --save in the packages we manage and add our dependencies directly.

That sounds painful, especially since npm install is pretty good at dealing with semver ranges.

I think if npm is meant to be the distribution tool then package.json should just go with what npm does.

agubler commented 8 years ago

Where did we get to on this as a standard for our package.json files? My two cents is that adhering to the standards from npm seems sensible... If npm provided a mechanism to override spaces vs tabs then I don't think it would be an issue but as that seems unlikely then I'm pro adopting spaces.

kitsonk commented 8 years ago

It is an open discussion. We have +2 for spaces and +1 for tabs. I assigned it out to others to try to get resolution, so it wasn't just @novemberborn and I glaring at each other.

Kagetsume commented 8 years ago

I personally like tabs over spaces because I can set my tab stops and make the indention appear however I like. It also makes the file smaller. On Jun 8, 2016 2:22 PM, "Kitson Kelly" notifications@github.com wrote:

It is an open discussion. We have +2 for spaces and +1 for tabs. I assigned it out to others to try to get resolution, so it wasn't just @novemberborn https://github.com/novemberborn and I glaring at each other.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dojo/dojo2-package-template/issues/29#issuecomment-224682639, or mute the thread https://github.com/notifications/unsubscribe/AAHpsYK-FWJUHCfqn7Wn4Z5bbK9ZL6wuks5qJwhxgaJpZM4IKe_z .

kfranqueiro commented 8 years ago

TL;DR: +1 tabs.

To me this would depend on which we expect to do more: npm i foo --save, or edit package.json manually. If the latter, which I would imagine to be the case, I'd be inclined to vote for hard tabs to match the rest of the repo.

You can always manually add deps to package.json as well, which I tend to do so I can do what I feel like in terms of version ranges anyway.

dylans commented 8 years ago

TL;DR: +1 tabs for me as well

agubler commented 8 years ago

I use --save or --save-dev regularly, not adopting the standard from an echosytem we use and rely on seems awkward.

dylans commented 8 years ago

I prefer the consistency of everything we create being tabs. I don't like being told that we must conform to their "standard" because they don't feel that we deserve a choice in something that should be a configuration setting and that all modern IDEs and editors can seamlessly switch as needed.

dylans commented 8 years ago

That said, I decided to read the other comments. I guess I pretty much agree with what Kit said earlier, in spite of my own preferences.

nicknisi commented 8 years ago

To me, it seems silly to prohibit functionality (--save and --save-dev, for example) because of ideology. This file won't be linted anyway, will it? Alternatively, we could look deeper into some solution to automatically rewrite the file, such as git hooks or filters.

kitsonk commented 8 years ago

Wow, it would be great if we could get this level of input for non 🚲 🏠 issues 😆 but thanks for all the input.

Of course recently, the NodeJS community reiterated the love for all things spaces:

So while I listed myself a 👎 before, I was really 😑 and the comments here are that while we internally could control our use of package.json and fight against the 🌊 of npm that it doesn't get linted, it isn't part of our "code" (and when installed anyways, it gets rewritten), only to have to spend time creating and maintain a git hook

So at the moment, I will keep the issue open for a few more days, just incase anyone wants to add their two pence, but it seems like we are leaning towards ¯_(ツ)_/¯ npm letting it be silly and moving on with our lives.

gcbw commented 8 years ago

the only solution to this problem is to send a patch to npm to have a space/tabs option/command line flag. or to use whatever is on the first indented line (but automagic things are always worse).

anyway, discussing what is better or worse will NEVER satisfy everyone.

that said, +1 for tabs. +1 for vim (since we are talking about personal preferences anyway :)

kfranqueiro commented 8 years ago

Except npm doesn't seem at all interested in that patch.

The only reasonable explanation/excuse I've seen as to why not is in this comment.

kitsonk commented 8 years ago

@gcbw, as @kfranqueiro points out the only reason we are debating it is because npm refuses to allow itself to be fixed.

While I might be with Richard Hendricks on this one, we do have two space indent in our .yml file and unlike Richard, I am not willing to lose my personal relationships over it.

So shall it be deemed that in the Great Whitespace Wars that in this battle we shall concede to npm and allow its automatic formatting to prevail for package.json in order to live to fight again another day.

✋ 🎤