Remove JSHint "ignore" rules for JSX Loop content

RESOLVED FIXED in Firefox 40


5 years ago
4 years ago


(Reporter: NiKo, Assigned: cedric.raudin, Mentored)



Firefox Tracking Flags

(firefox40 fixed)


(Whiteboard: [tech-debt][good first bug][lang=js])


(1 attachment)

Right now we have JSX code in this form:

render: function() {
  /* jshint ignore:start */
  /* jshint ignore:end */

We shouldn't do that, as:

- JSHint is not part of any build/validation step atm
- it prevents validating JSX and related transpiled code at all, which is error prone

It's up to developers to properly configure their editor/IDE, eg. using jsxhint[0] to get proper linting of JSX files.

Integrating JSXHint validation as part of the build process is probably an higher goal not part of this bug.

Whiteboard: [tech-debt]


5 years ago
backlog: --- → Fx36+


5 years ago
backlog: Fx36+ → Fx37+


4 years ago
backlog: Fx37+ → Fx38?


4 years ago
backlog: Fx38? → tech-debt
Priority: -- → P2
Setting as good first bug. The changes here are to remove the comments in the jsx files here:^[^\0]*%24&hitlimit=&tree=mozilla-central

(not in sdk.js).

Note that the js files are generated using jsx. See our developer documentation for more info:
Mentor: standard8
Whiteboard: [tech-debt] → [tech-debt][good first bug][lang=js]

Comment 2

4 years ago
Hi, I would like to work on this. I am new to open source contributions so it will be fine if someone could guide me through. I have already build Firefox
Hi Cedric, welcome!

As you've already got Firefox built, that's a good start. If you read comment 1 you need to edit the source to remove the lines highlighted in that link, i.e. the ones that are:

 /* jshint ignore:start */


 /* jshint ignore:end */

However, don't do this in the /browser/components/loop/content/shared/libs/sdk.js file. That's a library supplied by a third-party, so we don't alter that.

Once you've done the removals, its a good idea to check that Firefox still runs and that the Firefox Hello panel opens, as does the conversation window (click the Hello button on the toolbar and start a conversation.

Btw, which platform are you working on?

Once you've done that, there's details on how to submit a patch here:

If you attach the patch to the bug, and when you do so, set the review flag to "?" and put ":standard8" in the field it displays, that will then come through to me so I can check it.

If you get stuck or need help, feel free to just ask again or email me direct.
Assignee: nobody → cedric.raudin

Comment 4

4 years ago
Thanks for your informations.

My platform is: Windows 7, 64bits

Comment 5

4 years ago
Attachment #8594976 - Flags: review?(standard8)
Comment on attachment 8594976 [details] [diff] [review]

Thanks Cedric, this is just right. r=Standard8
Attachment #8594976 - Flags: review?(standard8) → review+
(In reply to Pulsebot from comment #7)

As you'll see from that, I just pushed the change for you. It goes to one of our integration trees and it should be merged to the main line (mozilla-central) within the next 24 hours, at which point this bug will be updated and marked as fixed.

Thanks for the patch, and congratulations on your first commit.
Target Milestone: --- → mozilla40
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.