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

de-jsonify default.js #2445

Merged
merged 1 commit into from
Jun 6, 2017
Merged

de-jsonify default.js #2445

merged 1 commit into from
Jun 6, 2017

Conversation

brendankenny
Copy link
Member

default.js was once default.json, and still has the double quotes to prove it. This updates to remove those, add a license header, enable eslint, and sprinkle in some trailing commas to hopefully help with git blame in the future while this commit is ruining it in the present :)

Disabled max-len for this file because of the description strings and this is a config file, not a normal script file, but happy to turn back on if others want it.

part of #2276

@paulirish paulirish mentioned this pull request Jun 6, 2017
7 tasks
Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

I formatted it with prettier and ended up with very similar output. LGTM

{"id": "pwa-page-transitions", "weight": 0, "group": "manual-pwa-checks"},
{"id": "pwa-each-page-has-url", "weight": 0, "group": "manual-pwa-checks"}
categories: {
'pwa': {
Copy link
Member

Choose a reason for hiding this comment

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

you can unquote these category titles

Copy link
Member Author

Choose a reason for hiding this comment

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

you can unquote these category titles

eslint doesn't like because best-practices is on the same level and needs quotes

@@ -1,303 +1,321 @@
/* eslint-disable */
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: small headers now :)

Copy link
Member Author

Choose a reason for hiding this comment

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

nit: small headers now :)

lulz I was hoping I'd get merged first :)

@brendankenny brendankenny merged commit 8275b6e into master Jun 6, 2017
@brendankenny brendankenny deleted the dejsonify branch June 6, 2017 02:20
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.

3 participants