1564300 - Old ESLint errors may be raised after newer pushes on Phabricator

Status: NEW

(Developer Infrastructure :: Source Code Analysis, defect, P3)

Description

[Marcos Caceres [:marcosc]]

Phabricator URL: https://phabricator.services.mozilla.com/D36885

ESlint on review bot was complaining about "Error: Insert · [eslint: prettier/prettier]". However, doing ./mach eslint locally on the affected file is not picking up the issue.

The original line was:

const manifest = ManifestProcessor.process({ ...args, ...aOptions });

I've had to change it to make ESLint happy on the sever.

Could it be that ESLint on the server is a different version than the one I have locally?

I tried updating by:

./mach  eslint -s

But it didn't help.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:sylvestre, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Victor, does it ring a bell?

Comment 3

[Victor Porof [:vporof][:vp]]

Eslint on the server should be the same version as the one mach installs locally. I think Mark can confirm for sure.

Comment 4

[Mark Banner (:standard8)]

I couldn't reproduce locally, so I've just checked through the history.

Diff 8 (128420) is the one that failed the lint job.

In this diff, the line was:

const manifest = ManifestProcessor.process({...args, ...aOptions });

With the error TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/dom/manifest/ManifestObtainer.jsm:120:47 | Insert·(prettier/prettier)

Diff 9 (128423) fixed this:

https://phabricator.services.mozilla.com/D36885?vs=128420&id=128423#toc

Diff 8 was a "fix merge screwup", Diff 9 was also titled "fix merge screwup". Diff 10 was "Make eslint happy".

Diff 9 was pushed about 5 minutes after diff 8. The ESLint issue was reported 37 minutes after Diff 8 was submitted.

So this looks like a case of crossed wires.

Sylvestre, maybe analysis tasks should be cancelled/ignored if they're for an older push?

Comment 5

[Andi [:andi]]

We've had this issue before, but aren't the comments grayed out? I'm not sure we have the possibility just yet to do what comment #4 implies.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Yeah, we have that on our todo list https://github.com/mozilla/code-review/issues/18 but this isn't a top priority.