Closed Bug 1479513 Opened 6 years ago Closed 2 years ago

[meta] Fix instances of uses of undefined constants in IDL files

Categories

(Toolkit :: General, task, P3)

Product:
Toolkit
Component:
General
Type:
task

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: standard8, Unassigned)

References

Details

(Keywords: meta)

Attachments

(1 obsolete file)

It was noticed in a couple of bugs that we were attempting to access constants in IDL files that are undefined. I whipped up a rule using ESLint (which can't land in its current state), that found a bunch of places where we're trying to access incorrect items. This bug is to track the fixing of those issues.
Depends on: 1477539, 1476555
Depends on: 1479726
Depends on: 1479727
Depends on: 1479728
Depends on: 1479730
Depends on: 1479731
Depends on: 1479732
(In reply to Mark Banner (:standard8) from comment #0) > It was noticed in a couple of bugs that we were attempting to access > constants in IDL files that are undefined. > > I whipped up a rule using ESLint (which can't land in its current state), > that found a bunch of places where we're trying to access incorrect items. > > This bug is to track the fixing of those issues. Can you put up the script somewhere? Do you have a plan of how to integrate it into CI so we don't reintroduce issues like these?
Flags: needinfo?(standard8)
Flags: needinfo?(standard8)
Attached patch Not commitable IDL constants checker (obsolete) (deleted) — — Splinter Review
(In reply to :Gijs (he/him) from comment #1) > Can you put up the script somewhere? Attached. This runs as part of ESLint, but relies on a build to have been created first, so that we have the xpt information. It also assumes my object directory path. I'm also not sure it'd work in an artifact build... > Do you have a plan of how to integrate it into CI so we don't reintroduce issues like these? Not really. I don't think it is really ESLint's concern to actually test this, so I think it would need a different analyser of some form. ESLint was just a useful tool that I could whip up a prototype that gave us answers. Currently I'm just running it occasionally to check for any new issues.
Assignee: nobody → standard8
Assignee: standard8 → nobody
Keywords: meta
Depends on: 1515615
Depends on: 1515616
Priority: -- → P3
Depends on: 1534827
Depends on: 1534830
Depends on: 1542842
Depends on: 1552535
Depends on: 1559084
Depends on: 1559087
Depends on: 1559088
Depends on: 1575477
Depends on: 1597653
Depends on: 1654341

Through some recent new performance tests, I found out about cron.yml. Could we use that to run a job that checks this once a day, as tier-2, so that the job of auditing this doesn't fall on you, and instead we can back out patches that add new instances? :-)

Flags: needinfo?(standard8)
Depends on: 1666491

(In reply to :Gijs (he/him) from comment #3)

Through some recent new performance tests, I found out about cron.yml. Could we use that to run a job that checks this once a day, as tier-2, so that the job of auditing this doesn't fall on you, and instead we can back out patches that add new instances? :-)

Yes, we should look at doing that, cron seems the right way. I've just filed bug 1666491 for the implementation.

Flags: needinfo?(standard8)
Depends on: 1666524
Depends on: 1701621
Depends on: 1704784
Depends on: 1716299
Depends on: 1716300
Depends on: 1762228
Depends on: 1762232
Depends on: 1787995

Comment on attachment 9002518 [details] [diff] [review]
Not commitable IDL constants checker

Moving patches to bug 1479515.

Attachment #9002518 - Attachment is obsolete: true
Type: enhancement → task
Blocks: 1666491
No longer depends on: 1666491
Severity: normal → S3
Depends on: 1795362
Depends on: 1795370

Marking as fixed - we now have a tier-2 builder that is watching for new instances (bug 1666491, bug 1800874) and they will be filed accordingly.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: