1160496 - Enable eslint rules for Loop: General code format

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P3)

Description

[Mark Banner (:standard8)]

Enable some more eslint rules for Loop. Rules to enable in this bug:

* curly (curly braces should be used)
* dot-notation (prefer dot notation over ["value"])
* eol-last (files should always end with a new line)
* space-infix-ops (spaces around operators)
* space-return-throw-case (space after return/throw/case)
* yoda (literal value should be on right of condition)

To enable the rule, remove the line from browser/components/loop/.eslintrc and save it.

Then you can run eslint in the browser/components/loop directory with:

  eslint  --ext .js --ext .jsm --ext .jsx .

The errors listed in the output are to be fixed.

If there's errors in a .js file that has an associated .jsx file, then fix the jsx file and run the react tools to generate the .js file (https://wiki.mozilla.org/Loop/Development#Developing).

If you need more help setting up eslint, see https://wiki.mozilla.org/Loop/Development#Additional_Requirements

Information about the eslint rules can be found here:

http://eslint.org/docs/rules/

Comment 1

[Martin Tomes [:tomesh]]

Hi, I would like to start working on this bug. Can you please assign me?

Comment 2

[Martin Tomes [:tomesh]]

Attachment: rev 1 - eslint general code format errors corrected.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8602712 [details] [diff] [review]
rev 1 - eslint general code format errors corrected.

Review of attachment 8602712 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good. Unfortunately there's quite a bit of whitespace on end of lines that needs tidying up before we can land this.

The review tool in bugzilla is quite good at showing it.

::: browser/components/loop/content/js/roomViews.jsx
@@ +306,5 @@
>      },
>  
>      render: function() {
> +      if (!this.state.show) { 
> +        return null; 

nit: The review tool is showing whitespace on end of line here.

::: browser/components/loop/content/shared/js/validate.js
@@ +29,5 @@
>     * @return {String}
>     */
>    function typeName(obj) {
> +    if (obj === null) { 
> +      return "null"; 

nit: more trailing whitespace in this file (several places).

::: browser/components/loop/modules/LoopCalls.jsm
@@ +323,5 @@
>     * @return true if the call is opened, false if it is not opened (i.e. busy)
>     */
>    startDirectCall: function(contact, callType) {
> +    if ("id" in this.conversationInProgress) { 
> +      return false; 

nit: trailing whitespace

::: browser/components/loop/modules/MozLoopService.jsm
@@ +724,5 @@
>     * @returns {Map} a map of element ids with localized string values
>     */
>    get localizedStrings() {
> +    if (gLocalizedStrings.size) { 
> +      return gLocalizedStrings; 

nit: trailing whitespace

::: browser/components/loop/test/desktop-local/conversationViews_test.js
@@ +54,5 @@
>        send: function() {},
>        abort: function() {},
>        getResponseHeader: function(header) {
> +        if (header === "Content-Type") { 
> +          return "audio/ogg"; 

nit: trailing whitespace in this file.

::: browser/components/loop/test/desktop-local/panel_test.js
@@ +386,5 @@
>  
>        beforeEach(function() {
>          supportUrl = "https://example.com";
>          navigator.mozLoop.getLoopPref = function(pref) {
> +          if (pref === "support_url") { 

nit: trailing whitespace

::: browser/components/loop/test/mochitest/head.js
@@ +247,5 @@
>      }, 30000);
>  
>      tab.linkedBrowser.addEventListener(eventType, handle, true, true);
> +    if (url) { 
> +      tab.linkedBrowser.loadURI(url); 

nit: trailing whitespace

::: browser/components/loop/test/shared/views_test.js
@@ +33,5 @@
>        send: function() {},
>        abort: function() {},
>        getResponseHeader: function(header) {
> +        if (header === "Content-Type") { 
> +          return "audio/ogg"; 

nit: trailing whitespace

::: browser/components/loop/test/standalone/webapp_test.js
@@ +40,5 @@
>        send: function() {},
>        abort: function() {},
>        getResponseHeader: function(header) {
> +        if (header === "Content-Type") {
> +          return "audio/ogg"; 

nit: trailing whitespace

Comment 4

[Martin Tomes [:tomesh]]

Ok. Im few days off now. I'll check it on Monday :)

Comment 5

[Martin Tomes [:tomesh]]

Attachment: rev 2 - eslint general code format errors corrected. trailings spaces removed

Comment 6

[Mark Banner (:standard8)]

Attachment: rev 3 - fixed bitrot

Thanks for the patch Martin. It looks great. There was a little bit of bitrot due to some changes we landed late last week, and I noticed that conversationViews_test.js still had some trailing whitespaces.

They were all simple, so I've gone ahead and fixed those, so this is ready to land now - I'll do it in just a moment.

Thanks!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3f78f06e37f6

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3f78f06e37f6