FLUID-5914: Fix linting error in .eslintrc...

Metadata

Source
FLUID-5914
Type
Task
Priority
Major
Status
Closed
Resolution
Fixed
Assignee
Antranig Basman
Reporter
Tony Atkins [RtF]
Created
2016-06-07T05:48:52.627-0400
Updated
2016-07-08T09:08:01.941-0400
Versions
N/A
Fixed Versions
N/A
Component
N/A

Description

The new .eslintrc file has a linting error (trailing comma). I will submit a quick PR to fix this, and also to add .eslintrc to the jsonlint checks.

Comments

  • Tony Atkins [RtF] commented 2016-06-07T05:58:49.236-0400

    As using the .eslintrc filename is deprecated, I will simply rename the file to .eslintrc.json, which will implicitly add it to our JSON linting rules.

  • Tony Atkins [RtF] commented 2016-06-07T06:07:58.582-0400

    The migration from using the JSHint "strict" setting to using strict: ["error", "function"] causes problems with node code. Even if the environment is set correctly, ESLint reports errors with node code that has global "use strict" statements. In reading the documentation, it seems like string: ["error", "safe"] is more appropriate, as it correctly changes the behavior depending on the environment.

    cc: @@Antranig Basman @@Cindy Li

  • Tony Atkins [RtF] commented 2016-06-07T06:54:20.463-0400

    @@Antranig Basman and @@Cindy Li, we should discuss this briefly today. In short, the current strict: ["error", "function"] setting will complain unless you wrap all of your node code in a function. I have tried the "safe" setting, it will work for code like that used in node-based tests, Kettle, gpii-express, but will fail for infusion itself, because the two node files here use a wrapper function.

    As far as I can see, the options are:

    1. "function" - works for browser code, requires changes to existing node code
    2. "global" - works for node code, requires changes to existing browser code
    3. "safe" - Works for everything except node code that already uses a wrapper function.

    I would argue for setting it to "safe" and making the "use strict" statements global in the node tests and fluid.js (lowercase, i.e. what is "require"d from node).

    If we need to use wrapper functions for some reason, we should discuss how to balance the default settings with things like Kettle and gpii-express, which definitely will not pass the current settings. In that case, my proposal would be to add comments in the node tests and fluid.js to disable the warning.

  • Tony Atkins [RtF] commented 2016-06-08T10:19:18.082-0400

    @@Antranig Basman and I discussed the new "safe" setting, and I have tested this with gpii-express. The only setting most people should need to adjust from the updated standard file is the "env" setting.