Add ESLint and Prettier formatting and run initial formatting #95

davidhemphill · 2017-11-04T05:44:29Z

In response to #93, this PR establishes the use of ESLint using Prettier for code formatting. The ESLint config is using https://github.com/postcss/eslint-config-postcss, and Prettier is configured using the following settings:

"semi": false, // Use semicolons
"singleQuote": true, // Use single quotes instead of double quotes
"printWidth": 80, // Limit each line to 80 characters
"tabWidth": 2, // Each tab is 2 spaces wide
"useTabs": false, // Use spaces instead of tabs
"trailingComma": "none", // Don't use trailing commas, even if allowed in things like arrays
"bracketSpacing": true // Use spaces between brackets

MichaelDeBoey · 2017-11-04T14:59:54Z

.eslintrc

+        "printWidth": 80,
+        "tabWidth": 2,
+        "useTabs": false,
+        "trailingComma": "none",


Why did you choose to remove the trailing comma's?
It makes cleaner diffs, which is always nicer I think?

adamwathan · 2017-11-04T16:45:19Z

Sweet, excited to get this in! I agree with @MichaelDeBoey I think we should force dangling commas for multiline though, which looks like an easy tweak to the config?

EDIT: I only actually care about this if our whole config is gonna end up being custom; I'd also be fine with just following something like Standard which forbid trailing commas.

davidhemphill · 2017-11-04T17:40:28Z

Good with me! I usually leave them in too.

…

On Sat, Nov 4, 2017 at 11:45 AM Adam Wathan ***@***.***> wrote: Sweet, excited to get this in! I agree with @MichaelDeBoey <https://github.com/michaeldeboey> I think we should force dangling commas for multiline though, which looks like an easy tweak to the config? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#95 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADmWor0hcjBTKN-LtGywxajMt77WSCvks5szJShgaJpZM4QR5sR> .

adamwathan · 2017-11-04T19:18:42Z

Can we also add the quote-props rule set to consistent? I hate seeing some quoted keys and some not in objects where we need to use numbers as keys or dashes in keys.

davidhemphill · 2017-11-05T02:17:45Z

This is where we'd have to jump off the Prettier wagon. They're not interested in adding this option yet: prettier/prettier#838.

MichaelDeBoey

Maybe it's best to include a trailing comma everywhere it's possible, so we have cleaner diffs in the future?

I'm sorry for the massive amount of comments (164, wtf! 😕) btw, but wanted to make sure you didn't forget any of them 🙂

MichaelDeBoey · 2017-11-05T17:12:57Z

__tests__/defineClass.test.js

-  let output = defineClass('smooth', {'-webkit-font-smoothing': 'antialiased'})
-  expect(c(output.toString())).toEqual(`.smooth { -webkit-font-smoothing: antialiased }`)
+  let output = defineClass('smooth', {
+    '-webkit-font-smoothing': 'antialiased'


Here we can add one

MichaelDeBoey · 2017-11-05T17:13:20Z

__tests__/defineClasses.test.js

-    'flex': {
-      display: 'flex',
+    flex: {
+      display: 'flex'


This one can be re-added I guess

MichaelDeBoey · 2017-11-05T17:14:35Z

__tests__/defineClasses.test.js

    'inline-flex': {
-      display: 'inline-flex',
-    },
+      display: 'inline-flex'


This one too

MichaelDeBoey · 2017-11-05T17:14:41Z

__tests__/defineClasses.test.js

-      display: 'inline-flex',
-    },
+      display: 'inline-flex'
+    }


This one too

MichaelDeBoey · 2017-11-05T17:15:49Z

src/build.js

          from: `./css/${filename}.css`,
          to: `./dist/${filename}.css`,
-          map: { inline: false },
+          map: { inline: false }


This one too

MichaelDeBoey · 2017-11-05T17:41:33Z

src/util/defineClass.js

    return postcss.decl({
      prop: `${property}`,
-      value: `${value}`,
+      value: `${value}`


This one too

MichaelDeBoey · 2017-11-05T17:41:45Z

src/util/defineClass.js

-  }).append(decls)
+  return postcss
+    .rule({
+      selector: `.${escapeClassName(className)}`


This one too

MichaelDeBoey · 2017-11-05T17:42:04Z

src/util/focusable.js

-  }).append(cloneNodes(rules))
+  return postcss
+    .atRule({
+      name: 'focusable'


This one too

MichaelDeBoey · 2017-11-05T17:42:11Z

src/util/hoverable.js

-  }).append(cloneNodes(rules))
+  return postcss
+    .atRule({
+      name: 'hoverable'


This one too

MichaelDeBoey · 2017-11-05T17:42:19Z

src/util/responsive.js

-  }).append(cloneNodes(rules))
+  return postcss
+    .atRule({
+      name: 'responsive'


This one too

MichaelDeBoey · 2017-11-05T17:45:16Z

package.json

  "version": "0.1.3",
-  "description": "A utility-first CSS framework for rapidly building custom user interfaces.",
+  "description":
+    "A utility-first CSS framework for rapidly building custom user interfaces.",


Maybe it's best to change the printWidth to 100 or so, so this one can be on 1 line?

MichaelDeBoey · 2017-11-05T17:45:37Z

src/lib/registerConfigAsDependency.js

-    throw new Error(`Specified Tailwind config file "${configFile}" doesn't exist.`)
+  if (!fs.existsSync(configFile)) {
+    throw new Error(
+      `Specified Tailwind config file "${configFile}" doesn't exist.`


This one could also be on one line again.

adamwathan · 2017-11-05T19:27:49Z

.eslintrc

        "tabWidth": 2,
        "useTabs": false,
-        "trailingComma": "none",
+        "trailingComma": "es5",


What's the difference between this and "comma-dangle": "always-multiline"?

https://eslint.org/docs/rules/comma-dangle#options

They're the same. Always add trailing commas when using multiple lines, and not when on a single line.

// Doesn't get trailing comma let contributors: ['adam', 'jonathan', 'steve', 'david'] // Trail those commas! let contributors: [ 'adam', 'jonathan', 'steve', 'david', ]

adamwathan · 2017-11-05T19:29:12Z

@MichaelDeBoey No need to point out each comma; once we get the ESLint and Prettier config set the way we want it will do all of that automatically; @davidhemphill didn't make any of these changes by hand it's just the result of Prettier running with the config that was in place at the time of running it.

MichaelDeBoey · 2017-11-05T19:47:45Z

@adamwathan Oh I didn't know that, I'm sorry 😕
I thought he did it all by hand so...

adamwathan · 2017-11-05T19:54:30Z

@MichaelDeBoey Haha all good! 😊

…e codebase

davidhemphill changed the title ~~Add ESLinting and Prettier formatting and run initial formatting~~ Add ESLint and Prettier formatting and run initial formatting Nov 4, 2017

davidhemphill changed the title ~~Add ESLint and Prettier formatting and run initial formatting~~ Add ESLint and Prettier formatting and run initial formatting [WIP] Nov 4, 2017

davidhemphill changed the title ~~Add ESLint and Prettier formatting and run initial formatting [WIP]~~ Add ESLint and Prettier formatting and run initial formatting Nov 4, 2017

MichaelDeBoey reviewed Nov 4, 2017

View reviewed changes

MichaelDeBoey suggested changes Nov 5, 2017

View reviewed changes

adamwathan reviewed Nov 5, 2017

View reviewed changes

David Hemphill and others added 6 commits November 6, 2017 15:00

Add ESLinting and Prettier formatting and run an initial format on th…

ec60796

…e codebase

fix eslint issues

9dbf588

Update Prettier config to use trailing commas

c404c55

Reformat files

79a2bb3

change print width to 100 characters

06ed308

Fix ESLint issues

bab36e0

adamwathan force-pushed the Add-ESLint-and-Prettier branch from 75f9c93 to bab36e0 Compare November 6, 2017 20:08

adamwathan merged commit 05b5bbd into master Nov 6, 2017

adamwathan deleted the Add-ESLint-and-Prettier branch November 6, 2017 20:10

davidhemphill mentioned this pull request Nov 7, 2017

ESLint for collaborators #93

Closed

Uh oh!

Add ESLint and Prettier formatting and run initial formatting #95

Add ESLint and Prettier formatting and run initial formatting #95

Uh oh!

Conversation

davidhemphill commented Nov 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamwathan commented Nov 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidhemphill commented Nov 4, 2017 via email

Uh oh!

adamwathan commented Nov 4, 2017

Uh oh!

davidhemphill commented Nov 5, 2017

Uh oh!

MichaelDeBoey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamwathan commented Nov 5, 2017

Uh oh!

MichaelDeBoey commented Nov 5, 2017

Uh oh!

adamwathan commented Nov 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidhemphill commented Nov 4, 2017 •

edited

Loading

adamwathan commented Nov 4, 2017 •

edited

Loading