1180343 - Add notification when eslint throws an error

Status: RESOLVED DUPLICATE of bug 1229588

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Like bug 1180341 for android lint:

So we can enforce the lint results and ensure we don't regress issues we've already fixed.

Ideally, this would be Treeherder but it may take a lot of effort to do that so we should look into alternatives. Ideas:
  * (Like Hello) Cron job that pulls fx-team every few hours, runs lint, and sends an email if it fails
  * Run lint locally on every build (perf impact...?)
  * Another CI system

Let's see if Nick has opinions in bug 1180341.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: `eslint --setup` Task cluster config via web interface

I played around w/ TaskCluster and came up with the attached config to run via the testing web interface.

Some considerations:
* We should figure out how to not pull a different copy of the source
* There should be a `--setup` task, which blocks a mobile eslint task
* We should make this run on every push that modifies our js files
* There are a lot of other configuration options that I don't know what they do

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

Running at build time is better than what we have now but I think it's preferable to pull this earlier and catch issues before they are committed to the tree. This means possibly and Hg hook (if we don't noticeably slow down commits), integration with MozReview to prevent r+ on bad patches and block commits via autoland, and integration (for quick running linters) into dev tools.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Likely dupe: bug 1229588. Hence soft depends on.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Looks like this was fixed with bug 1229588 – our eslintignore was copied to the root level. Yay!

Unclear what happens when the TC job fails though – does the push get backed out? Or does the job just go red? I'd definitely like to make sure there's accountability here.

Comment 5

[:Gijs (he/him)]

(In reply to Michael Comella (:mcomella) from comment #4)
> Looks like this was fixed with bug 1229588 – our eslintignore was copied to
> the root level. Yay!
> 
> Unclear what happens when the TC job fails though – does the push get backed
> out? Or does the job just go red?

People get backed out for breaking the [ES] job if the sheriff isn't nice and fixes things for them (which is sometimes doable when it's just things like "no line end at the end of a file").