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

Quotes #10709

Merged
merged 1 commit into from
Dec 8, 2013
Merged

Quotes #10709

merged 1 commit into from
Dec 8, 2013

Conversation

XhmikosR
Copy link
Member

No description provided.

@cvrebert
Copy link
Collaborator

Per the Bootstrap JavaScript code style guidelines, semicolons should not be used.

@XhmikosR
Copy link
Member Author

I missed that, it seems. I'll update the PR.

@@ -34,6 +34,9 @@ module.exports = function(grunt) {
},
test: {
src: ['js/tests/unit/*.js']
},
assets: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Explanation? This isn't quote-related.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the Bootstrap files from the assets folder in the JSHint task.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this patch to my other PR so that this PR has only the quotes change.

@zlatanvasovic
Copy link
Contributor

👍

@XhmikosR
Copy link
Member Author

Any news on this?

@zlatanvasovic
Copy link
Contributor

@cvrebert ping

2013/9/25 XhmikosR notifications@github.com

Any news on this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/10709#issuecomment-25071935
.

Zlatan Vasović - ZDroid

@cvrebert
Copy link
Collaborator

This is in @fat's domain.

@zlatanvasovic
Copy link
Contributor

Oh, yea. But I think that this can be merged, because jQuery styleguide
says single quotes, except for JSON. :)

2013/9/25 Chris Rebert notifications@github.com

This is in @fat https://github.com/fat's domain.


Reply to this email directly or view it on GitHubhttps://github.com//pull/10709#issuecomment-25105953
.

Zlatan Vasović - ZDroid

@cvrebert cvrebert closed this Sep 25, 2013
@cvrebert cvrebert reopened this Sep 25, 2013
@cvrebert
Copy link
Collaborator

Gah, misclicked.
We don't use the jQuery style guide. We have our own.

@zlatanvasovic
Copy link
Contributor

:/

2013/9/25 Chris Rebert notifications@github.com

Gah, misclicked.
We don't use the jQuery style guide. We have our own.


Reply to this email directly or view it on GitHubhttps://github.com//pull/10709#issuecomment-25107984
.

Zlatan Vasović - ZDroid

@XhmikosR
Copy link
Member Author

PR rebased.

@zlatanvasovic
Copy link
Contributor

@XhmikosR Two core faults in your pull requests are:

  • JSON (like the metadata in Gruntfile) use double quotes
  • "use strict"; is used in Bootstrap, so no single quotes

@XhmikosR
Copy link
Member Author

I don't have a preference; it's just that atm quotes aren't being used consistently. That's what this patch is for.

@mdo
Copy link
Member

mdo commented Nov 30, 2013

Based on the number of changes, it looks like the double quotes are used more than single quotes. Thanks, but we'll leave as-is and can update the single quotes in a separate PR.

@mdo mdo closed this Nov 30, 2013
@XhmikosR
Copy link
Member Author

@mdo: quite the opposite. Single quotes are used in the codebase, that's why I went with it.

@mdo mdo reopened this Nov 30, 2013
@XhmikosR
Copy link
Member Author

XhmikosR commented Dec 8, 2013

@mdo: Rebased against the latest master.

mdo added a commit that referenced this pull request Dec 8, 2013
@mdo mdo merged commit b5613c9 into twbs:master Dec 8, 2013
@mdo mdo mentioned this pull request Dec 8, 2013
@XhmikosR XhmikosR deleted the quotes branch December 8, 2013 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants