Closed Bug 999737 Opened 8 years ago Closed 7 years ago

Add ESLint to the build & test process for Loop

Categories

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

defect
Points:
2

Tracking

(firefox40 fixed)

RESOLVED FIXED
mozilla40
Iteration:
40.1 - 13 Apr
Tracking Status
firefox40 --- fixed
Blocking Flags:
backlog tech-debt

People

(Reporter: pdehaan, Assigned: dmosedale)

Details

(Whiteboard: [tech-debt])

Attachments

(2 files, 1 obsolete file)

Thanks for the report. I don't see anything major here, so I think we can leave these tidy ups until after we land something in nightly, when we'll also have a bit more context and time to decide what level of static checking (and which tool(s)) that we want going forward.
Severity: normal → trivial
OS: Mac OS X → All
Priority: -- → P5
Hardware: x86 → All
Whiteboard: [tech-debt]
backlog: --- → Fx36+
backlog: Fx36+ → Fx37+
Its a great idea, which we want to take further and get ESLint in the tree for at least Loop. We may start with jsxhint as an intermediary given our current use of react.

Therefore morphing the bug, and we'll fix the warnings/errors when we get it included.
Severity: trivial → normal
backlog: Fx37+ → tech-debt
Priority: P5 → P2
Summary: A few errors/warnings reported via ESLint against loop-client → Add ESLint to the build & test process for Loop
I've tagged comment 0 as obsolete as so many things have changed since that run.
Assignee: nobody → dmose
Comment on attachment 8586447 [details] [diff] [review]
bootstrap eslint for loop content JS, fix a found bug

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

rs=me Note that I don't know much about eslint but it seems like this is mostly getting the initial setup ready so we can start enforcing it more later.
Attachment #8586447 - Flags: review+
Indeed, this is simply bootstrapping the various files to ignore relevant things, and turning off most of the rules, so that we don't need to do a giant code-whack all-at-once to get things started.

That said, even these meager settings found one bug, and I've included a fix for that here.

To start taking advantage of this, all you have to do (once) is go into browser/components/loop/standalone and do "make install", which will npm install the eslint bits you need.

I've got some commented out bits in run-all-loop-browser-tests.sh which will start running these whenever someone executes that script and happens to have eslint installed locally, from the above step, which I'll get review on shortly.
Note also that your favorite editor/IDE may now, possibly with a bit of configuration, be able to give you instantaneous feedback based on the configuration files now in the tree. http://eslint.org/docs/user-guide/integrations has the details.
I've set up a cron job on the dev server which runs eslint on an updated copy of fx-team every hour, and sends me any error output.  If anyone else would like on the list, please let me know. :-)

Obviously, this is just for the bootstrapping phase; we'll soon want to find a faster and more robust feedback mechanism (eg travis-ci, treeherder, ...).  Spinoff bugs to come.  If you're interested in getting email for this when it fails, feel free to let me know.  :-)
(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #7)
> I've got some commented out bits in run-all-loop-browser-tests.sh which will
> start running these whenever someone executes that script and happens to
> have eslint installed locally, from the above step, which I'll get review on
> shortly.

Adding leave-open as it seems like this review is still pending, and I'm assuming that's going ot happen in this bug.
Keywords: leave-open
Iteration: --- → 40.1 - 13 Apr
Points: --- → 2
Keywords: leave-open
Target Milestone: --- → mozilla40
https://hg.mozilla.org/mozilla-central/rev/c6b002187f58
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
People interested in this ticket may be interested in Bug 1177933.
You need to log in before you can comment on or make changes to this bug.