Closed Bug 1572515 Opened 4 months ago Closed 3 months ago

mach lint complains about newtab logs missing license


(Firefox :: Messaging System, task, P2)




Firefox 70
70.4 - Aug 19 - Sep 1
Tracking Status
firefox70 --- fixed


(Reporter: Mardak, Assigned: Mardak)




(1 file)

Bug 1562645 turned on license linting of html which happens to include the code coverage logs pages:

  0   error  No matching license strings found in tools/lint/license/valid-licenses.txt  (license)

  0   error  No matching license strings found in tools/lint/license/valid-licenses.txt  (license)

Where these files are only generated when running npm test directly from mozilla-central workflow (bug 1552007).

Adding a plain - browser/components/newtab/logs to the exclude list works only when the directory exists:

LinterParseError: tools/lint/license.yml: The exclude directive contains the following paths that don't exist:

But that directory isn't checked in as it's "built"…

Assignee: edilee → nobody

Exclude "built" logs/ directory from any linting

Can logs be output in the object directory instead? That would be the ideal solution IMO.

dmose, perhaps as part of bug 1459773, have you found a good way to have the scripts behave differently depending on when they're run from mozilla-central vs activity-stream repositories ?

Flags: needinfo?(dmose)

I haven't yet needed to, so no.

That said, one hacky way to do it would be see if the parent path of the script contains browser/components/newtab. What script are you imagining tweaking in this case?

Flags: needinfo?(dmose)

I suppose it would be more of a change to detect and use a different coverageReportingPath: "logs/coverage/" and then updating npm run debugcoverage to look at the right place if it's in m-c vs a-s

Assignee: nobody → edilee
Iteration: --- → 70.3 - Aug 5 - 18
Priority: -- → P1

A relatively straightforward way to do the thing could look something like this:

  • write a tiny script that figures out whether it's in mc or not, perhaps using something like the hack above
  • use that script to set an npm variable (see npm help 7 npm-config), perhaps using a project .npmrc (hidden using .gitignore and .hgignore), or perhaps some other way
  • have and debugcoverage pull the path from the environment
Priority: P1 → P2
Iteration: 70.3 - Aug 5 - 18 → 70.4 - Aug 19 - Sep 1
Pushed by
mach lint complains about newtab logs missing license r=dmose
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.