Opened 2 years ago

Closed 2 years ago

#5560 closed change (fixed)

Specify the jsdoc dependency, instead of expecting it to be installed globally

Reported by: kzar Assignee: kzar
Priority: P3 Milestone:
Module: Automation Keywords:
Cc: trev, sebastian, tlucas Blocked By: #5559, #5596
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29531680/

Description (last modified by trev)

Background

With #5559 ensure_dependencies.py will install Node.js dependencies when a dependency is updated. So far we rely on JSDoc being installed globally, but now we can do things properly.

What to change

  • Create the file buildtools/package.json with the following contents:
{
  "name": "buildtools",
  "repository": "https://hg.adblockplus.org/buildtools",
  "license": "MPL-2.0",
  "dependencies": {
    "jsdoc": "3.5.4"
  },
  "scripts": {
    "jsdoc": "jsdoc"
  }
}
  • Modify buildtools/build.py to:
    • Remove the "This operation requires JsDoc 3 to be installed." text from the command description.
    • Call JSDoc as npm run-script jsdoc -- <jsdoc_arguments>.
  • Add ^node_modules$ and /node_modules/ to buildtools/.hgignore and buildtools/.gitignore respectively. (Make sure the regexp syntax is used for the entry in .hgignore.)

Change History (13)

comment:1 Changed 2 years ago by kzar

  • Description modified (diff)

comment:2 Changed 2 years ago by kzar

  • Description modified (diff)

comment:3 Changed 2 years ago by kzar

  • Description modified (diff)

comment:4 Changed 2 years ago by trev

  • Description modified (diff)

npm run-script doesn't run arbitrary commands, it only runs scripts specified in patterns.json. I updated the description with an approach that should work.

comment:5 Changed 2 years ago by trev

  • Description modified (diff)

comment:6 Changed 2 years ago by tlucas

  • Description modified (diff)

Minor change, the buildtools' license is MPL 2.0

comment:7 Changed 2 years ago by trev

  • Description modified (diff)

I corrected the license identifier, it should be "MPL-2.0" rather than "MPL 2.0" - see https://spdx.org/licenses/

comment:8 Changed 2 years ago by tlucas

As discovered during #5559, npm install (in version 8) generates a package-lock.json file. This should be committed along with the package.json.

comment:9 Changed 2 years ago by kzar

  • Blocked By 5596 added

comment:10 Changed 2 years ago by kzar

  • Owner set to kzar
  • Priority changed from Unknown to P3
  • Ready set

comment:11 Changed 2 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:12 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5560 - Specify the JSDoc dependency explicitly

comment:13 Changed 2 years ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.