HubSpot / eslint-config

HubSpot's ESLint configurations
1 stars 2 forks source link

JSDoc validation #7

Open TrevorBurnham opened 6 years ago

TrevorBurnham commented 6 years ago

@geekjuice In the latest master of HubSpot/javascript, the valid-jsdoc rule is disabled. But in the first commit of this repo, the valid-jsdoc rule is enforced as an error. Can you explain where that change came from?

geekjuice commented 6 years ago

IIRC, I probably removed it since not many people used JSDocs. Didn't see the need to enforce a rule that would be ignored most of the time 😬

TrevorBurnham commented 6 years ago

@geekjuice Other way around: The rule used to be turned off (as it is in Airbnb/javascript), but now it's turned on as an error (in eslint-config-hubspot 7.0.0+). I assume you copy-pasted this repo's initial errors.js from somewhere, but where?

I'll submit a PR to flip the rule back off.

geekjuice commented 6 years ago

Oh derp. I honestly don't remember. I know I based this version from Segments configs but I may have just turned it on by mistake :shrug:

Phoenixmatrix commented 6 years ago

Yeah, I wouldn't overthink this too much. We're probably moving to a barebone superset of eslint:recommended + a few more rules (along with Prettier), and people can add their own extra rules to their own project if they want to be more strict.

In the interim, if rules are not 100% beneficial here, we should just turn them off. The 3 people at HubSpot who strictly follow jsdoc can add the rule to their own eslintrc.

geekjuice commented 6 years ago

fwiw, here is the base config we've been using on our end:

{
  "parser": "babel-eslint",
  "parserOptions": {
    "ecmaFeatures": {
      "jsx": true
    }
  },
  "env": {
    "browser": true
    // `es6` intentionally omitted to prevent use of native Map, Set, List, etc.
  },
  "globals": {
    "Symbol": true // since we have polyfills
  },
  "extends": [
    "eslint:recommended",
    "plugin:react/recommended",
    "prettier",
    "prettier/react"
  ],
  "plugins": [
    "react",
    "prettier"
  ],
  "rules": {
    "react/display-name": "off", // plugin expects `displayName` for stateless...
    "prettier/prettier": "error"
  },
  "settings": {
    "react": {
      "version": "15.2.1"
    }
  }
}

additional rules/settings can then be defined in those subdirectories. i'm not suggesting these be the new rules for eslint-config-hubspot, rather wanted to just share a single data point.

geekjuice commented 6 years ago

just chatted with @Phoenixmatrix and it sounds like eslint-plugin-prettier could be omitted from the config i posted if we used prettier -l instead :themoreyouknow: