Enable eslint rules for Loop: General code format

RESOLVED FIXED in Firefox 41

Status

Hello (Loop)
Client
P3
normal
Rank:
38
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: standard8, Assigned: tomesh, Mentored)

Tracking

unspecified
mozilla41
Points:
---
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox41 fixed)

Details

(Whiteboard: [lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
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/
(Assignee)

Comment 1

3 years ago
Hi, I would like to start working on this bug. Can you please assign me?
(Reporter)

Updated

3 years ago
Assignee: nobody → tomesm

Updated

3 years ago
Rank: 38
Flags: firefox-backlog+
Priority: -- → P3
(Assignee)

Comment 2

3 years ago
Created attachment 8602712 [details] [diff] [review]
rev 1 - eslint general code format errors corrected.
Attachment #8602712 - Flags: review?(standard8)
(Reporter)

Comment 3

3 years ago
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
Attachment #8602712 - Flags: review?(standard8) → review-
(Assignee)

Comment 4

3 years ago
Ok. Im few days off now. I'll check it on Monday :)
(Assignee)

Comment 5

3 years ago
Created attachment 8604550 [details] [diff] [review]
rev 2 - eslint general code format errors corrected. trailings spaces removed
Attachment #8602712 - Attachment is obsolete: true
Attachment #8604550 - Flags: review?(standard8)
(Reporter)

Comment 6

3 years ago
Created attachment 8604628 [details] [diff] [review]
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!
Attachment #8604550 - Attachment is obsolete: true
Attachment #8604550 - Flags: review?(standard8)
Attachment #8604628 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/3f78f06e37f6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.