Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Templates: (JS/TS) change ' to " #360

Closed
wants to merge 5 commits into from
Closed

Templates: (JS/TS) change ' to " #360

wants to merge 5 commits into from

Conversation

stephtr
Copy link
Contributor

@stephtr stephtr commented Oct 5, 2016

I converted all quotes to double qoutes in ClientApp and webpack.config.* files

I have changed all ' to " in ClientApp and webpack.config.* files
@dnfclas
Copy link

dnfclas commented Oct 5, 2016

Hi @stephtr, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link

dnfclas commented Oct 5, 2016

@stephtr, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@MarkPieszak
Copy link
Contributor

MarkPieszak commented Oct 5, 2016

Hey Stephan, You're removing the placeholder text files that I believe Steve needs for the generators and the old tsd files that are currently necessary for the project until it's moved to TS2.

@stephtr
Copy link
Contributor Author

stephtr commented Oct 5, 2016

Oh sorry, I didn't want these two commits to be included into the pull request.

@MarkPieszak
Copy link
Contributor

MarkPieszak commented Oct 5, 2016

No worries just letting you know! :)
I'm checking to make sure the TS2 PR is all set with ReactRedux template and then all of them will have that stuff removed and updated as well!

@stephtr
Copy link
Contributor Author

stephtr commented Oct 5, 2016

Fixed it.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Oct 5, 2016

First, thanks @stephtr for submitting this. I appreciate the effort you've taken!

At the moment, I'm not sold on the idea of changing to ". I know what it says in TypeScript's coding guidelines, but:

  1. That guide tries to say that some parts of it are only "meant for contributors to the TypeScript project", whereas other parts "apply as to how idiomatic TypeScript code should be written (e.g. the "Names" and "Style" sections)" (which the "quotes" part is not in)
  2. Much more importantly (to such a degree that it renders item 1 almost irrelevant anyway), the JavaScript community and in turn the TypeScript community is nearly unanimous in using single quotes. All of Angular 2 is coded this way, and they document it that way, and their popular starter kits are the same. The official Angular 2 CLI also emits projects that use '.
  3. Among React apps, there isn't a single most-obvious authoritative TypeScript example, but all of the first few starter kits I've found (this, this, this) use '. I didn't find any that use " (though I stopped looking after those first three). Interestingly, React's official starter kit (linked from their main docs here), which uses JS not TS, consistently uses ' in JS code and " around HTML attribute values, so we could do the same in the two React templates.

Considering the degree of consistency in the community on preferring ', it seems like the TypeScript team's use of " in their own source code is more of a historical quirk, and if we used it, we'd look extremely out-of-step with the rest of the JS/TS community.

Now, as we all know, code style discussions (like naming discussions) tend to be extremely contentious and emotional. So at this point I don't really think it's a great idea to have a general discussion about people's preferences - it will just be frustrating to everyone. Instead I'm going to take the risk of closing this so we can judge in another month or so whether other people keep bringing it up. I know that will be frustrating to @stephtr so I apologise for that and thank you again for doing the work to create this PR. Let's see if we can stay consistent with the broader community rather than having yet another awkward cultural gap between Microsoft-focused devs and everyone else. Hope that's OK for now!

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

Successfully merging this pull request may close these issues.

4 participants