Opened on 08/23/2017 at 01:56:52 PM

Closed on 08/31/2017 at 12:36:10 PM

#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.)

Attachments (0)

Change History (13)

comment:1 Changed on 08/23/2017 at 02:13:16 PM by kzar

  • Description modified (diff)

comment:2 Changed on 08/23/2017 at 02:55:22 PM by kzar

  • Description modified (diff)

comment:3 Changed on 08/23/2017 at 03:01:17 PM by kzar

  • Description modified (diff)

comment:4 Changed on 08/23/2017 at 03:05:02 PM 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 on 08/23/2017 at 03:05:32 PM by trev

  • Description modified (diff)

comment:6 Changed on 08/24/2017 at 08:59:19 AM by tlucas

  • Description modified (diff)

Minor change, the buildtools' license is MPL 2.0

comment:7 Changed on 08/24/2017 at 09:09:46 AM 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 on 08/25/2017 at 10:23:54 AM 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 on 08/30/2017 at 02:24:33 PM by kzar

  • Blocked By 5596 added

comment:10 Changed on 08/30/2017 at 02:42:40 PM by kzar

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

comment:11 Changed on 08/30/2017 at 03:26:21 PM by kzar

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

comment:12 Changed on 08/31/2017 at 12:34:43 PM by abpbot

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

comment:13 Changed on 08/31/2017 at 12:36:10 PM by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from kzar.
 
Note: See TracTickets for help on using tickets.