Code Formatting - For Discussion Only

[anthonysena]

NOTE: This PR is only for review and should not be merged.

Per our ATLAS WG call today, I've made an initial attempt to implement EditorConfig and ESLint rules for ATLAS. The goal is to close out all open PRs for Atlas 2.7, implement these code formatting standards and then re-format the code base in a single PR so that moving forward we can avoid code formatting issues in subsequent PRs.

This aims to address #655 and is part of #990 (comment).

Work Notes

EditorConfig

• Added .editorconfig to the project and based it on vscode's project settings. Added some additional settings for ensuring the file format is UTF-8 and that there is a newline at the end of each file for making it nicer when scrolling to the bottom of a file
• Since I'm using VSCode as my IDE for ATLAS, I added the Editor Config for VS Code
• As a test, I then formatted js/const.js and js/settings.js and it fixed up the tabs/space issues we noted on the call today.

ESLint

• Added in eslint, to package.json via npm.

• Installed ESLint for VSCode which provides some nice shortcuts when working in VSCode.

• Using the VSCode plugin, I initialized the .eslintrc.json file. If you look at this file, you'll see the following choices that I made as part of the setup:

  1. Use ECMAVersion 6 when parsing code
  2. Use tabs for indentation otherwise throw an error
  3. Use "CRLF" (aka Windows) linebreak style
  4. Use quotes for strings (vs single quote)
  5. Use semicolons

• Opened up js/settings.js and ESLint flagged a number of problems. Most were easily remedied using the VSCode ESLint plugin which has a command to fix all auto-fixable problems. This left me with a few exceptions which I was able to override using the overrides configuration section in .eslintrc.json

• Next, I opened js/const.js and ESLint had trouble parsing the file. Per this SO article I installed and configured babel-eslint as the parser in .eslintrc.json. During the course of digging into this error, I also discovered an ESLint plugin for RequireJS and added that as well. I'm unsure if we need this but for now I've configured this to use the recommended ESLint settings for RequireJS.

• Once I took the steps above, I was able to auto-fix the ESLint errors in js/const.js via VSCode.

• I also took out .jshintrc (JSHint config) since I wasn't sure if that would cause any compatibility issues (nor am I sure if we still need this for linting if we go with ESLint instead?)

Next Steps

• Confirm we're ready and willing to use ESLint for our code.
• Gain agreement on the EditorConfig and ESLint rules above - this goes beyond the original "Tabs over Spaces" discussion.
• Confirm we're accepting of having exceptions in the .eslintrc.json as I did in this example or if there is another approach for selectively applying code formatting rules.
• Once we have consensus to move forward, we can take this PR the next step and reformat all of the code and address all ESLint rules that are flagged

Tagging: @pavgra, @chrisknoll, @fdefalco, @johnSamilin

[anthonysena]

From Atlas call discussion:

• Build process should utilize ESLint to enforce all coding conventions before code is included in the code base as a Travis CI Job (for future discussion and working session)
• Look to adopt the eslint-config-airbnb module so that we're not debating standards. Agreed to use a well-established module and run with it. I'll follow up on this point and post out the details

[vlbe]

1. I am not sure that * eslint-config-airbnb will be a good choice. I'll prefer to make our own set of rules(based on this or other config). This will be more flexible for us.
2. Let's make pre-commit hooks with prettier. Thus we can rid of awkard diffs with whitespaces, tabs/spaces etc.
3. I'll prefer spaces, andi'll be ok with tabs, but let's change indent size from 4 to 2. It will be easier to read/write code on small display screens.
4. I think it's better to use single quotes in js, and backticks with some variables. Let's distinguish somehow html from js. For html - double quotes, for js - single.