CodingTrain / website-archive

Archive of the Coding Train website (first version)
https://codingtrain.github.io/website-archive/
MIT License
5.72k stars 5.65k forks source link

Adding a linter #496

Open MeaningOf42 opened 6 years ago

MeaningOf42 commented 6 years ago

As @shiffman mentioned in #493, it would be a good idea to add a linter to this site to ensure only clean code is submitted. I would be willing to give this a go, but I have some questions about his comment:

I prefer spaces around => I guess I should add some sort of linter to this repo? Is AirBnB style guide what I should adopt and is there a linter that goes with it? This is new to me, but I would love to make a video about it so advice appreciated! @meiamsome

meiamsome commented 6 years ago

I linked Dan to ESLint's website as my suggestion for a linter as it is my preferred JS linter. The init of ESLint lets you chose from 3 default styles or create your own. The defaults are:

  1. Google
  2. AirBnB
  3. Standard

Note that Standard disallows semicolons at the end of lines, which is very contrary to @shiffman's style as I have seen it. I could not work out if Google or AirBnB matches closer. The other option is to just create an ESLint config manually, which is not terribly hard as ESLint defaults are fairly non-controversial. We also have to note that there is a mixture of Node and Browser code in this repository if we want to lint everyting, so we need to be careful about what we disallow (Some older style code produces warnings in the standard style).

MeaningOf42 commented 6 years ago

I dislike the idea of starting a new config from scratch as that would create a new style for people to learn. However if we made sure that all our decisions are compatible with one of the style defaults then, I would be ok creating a custom config file, as we could simply tell people it is a less strict version of the style it is compatible with. (Not sure if I was clear. My problem would be if we adopted some practices from Google and some from AirBnB as this would create a confusing mixture of the two.)

I think vanilla Google or vanilla AirBnB seem like the best options, but that is just my taste. I think it goes without saying that the style we end up adopting (if we follow through with the idea of adding a lint) should be similar to @shiffman's style. Seeing as he is (obviously) the person with the most experience with and intuition of his style it would be nice to hear his thoughts on the matter before starting work on linting.

As per the node vs browser code issue, we could potentially have two different styles for the two. However I think it would be best if we didn't do this, as part of the beauty of JavaScript is being able to use the same language for front and back end development, and enforcing two different styles for the two would break this nicety. (I'm not sure if the Google or AirBnB styles would cause issues if we enforced them for server and browser code, but I don't think so).

Versatilus commented 6 years ago

Considering his focus on p5js, it seems logical to me that he would adopt a style similar to the one they use. In my opinion, it's the most straightforward to adopt and understand, and most importantly, to make a video about. There's a lot of homework involved in using AirBnB or Google style guides. I will include below a slightly stripped-down version of what they use for p5. He could use it as a global default, with local settings customized for specific projects. The beautify package in Atom can be set to use the eslint fixer, which should make it easy to maintain consistency

.eslintrc.js:

module.exports = {
  env: {
    browser: true,
    amd: true,
    es6: true
  },
  extends: ['eslint:recommended', 'prettier'],
  plugins: ['prettier'],
  rules: {
    'prettier/prettier': [
      'error',
      {
        singleQuote: true
      }
    ],
    'no-cond-assign': [2, 'except-parens'],
    eqeqeq: ['error', 'smart'],
    'no-use-before-define': [
      2,
      {
        functions: false
      }
    ],
    'new-cap': 0,
    'no-caller': 2,
    'no-undef': 2,
    'no-unused-vars': ['error', { args: 'none' }],
    'no-empty': ['error', { allowEmptyCatch: true }],
    'no-console': 'off'
  }
};
shiffman commented 6 years ago

Thank you for this discussion! I have to admit I am flying a bit blind here as this is (embarrassingly) my first experience setting up my own linter. The good news is that my life's passion is indentation and spacing. I like @Versatilus of starting with the p5 style and adjusting as needed. I think it would be a good learning experience (and lead to a tutorial) if I try to set this up. I will take a look hopefully sometime this week. Any tips or further thoughts welcome!

jonnytest1 commented 5 years ago

maybe would be a good idea to only allow merges / pushes when the linter isnt complaining about anything . For those that want to go full force we could also add tslint with jsdoc type annotations oO even though that would probably break the "this." meme ...