Opened 3 years ago

Closed 3 years ago

#4047 closed change (fixed)

Improve [convert_js] metadata section syntax in adblockpluschrome build scripts

Reported by: kzar Assignee: sebastian
Priority: P3 Milestone:
Module: Automation Keywords: goodfirstbug
Cc: sebastian, jsonesen, kvas Blocked By: #4375
Blocking: #4376 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29350281

Description (last modified by sebastian)

Background

The build scripts for adblockpluschrome support a convert_js metdata section in order to transpile ES6 JavaScript code at buildtime. It works pretty well but the syntax is cumbersome, causing us problems when you want to add an input file to the list using the += syntax introduced with #2711.

What to change

With the current syntax, an example use of convert_js looks like this:

[convert_js]
lib/adblockplus.js = adblockpluscore/lib/events.js
  lib/prefs.js
  lib/utils.js
  ...
  --arg module=true source_repo=https://hg.adblockplus.org/adblockpluscore/

Update the buildscripts in buildtools so that the same example would instead look like this:

[convert_js]
lib/adblockplus.js = adblockpluscore/lib/events.js
  lib/prefs.js
  lib/utils.js
  ...
lib/adblockplus.js[module] = true 
lib/adblockplus.js[source_repo] = https://hg.adblockplus.org/adblockpluscore/

Also update the buildtools dependency in adblockpluschrome, along with any metadata files where the convert_js section is used.

Notes

  • The arguments module and source_repo are just examples, they should not be hardcoded, rather any arguments should be supported with the syntax.
  • The +=/-= syntax used with metadata inheritance must work for the convert_js options. For example we might want to do lib/adblockplus.js += some/safari/specific/file.js or even lib/adblockplus.js:someOption += something.

Change History (11)

comment:1 Changed 3 years ago by sebastian

  • Description modified (diff)

comment:2 Changed 3 years ago by sebastian

  • Cc jsonesen added
  • Keywords goodfirstbug added
  • Priority changed from Unknown to P3
  • Ready set

comment:3 Changed 3 years ago by sebastian

  • Description modified (diff)

comment:4 Changed 3 years ago by sebastian

  • Description modified (diff)

comment:5 Changed 3 years ago by sebastian

  • Cc kvas added

Jon and Vasily, does any of you'd like to work on this?

comment:6 Changed 3 years ago by sebastian

  • Owner set to sebastian

comment:7 Changed 3 years ago by sebastian

  • Description modified (diff)

Apparently, ConfigParser is considering colons (:) as separators just like = characters. So I had to change the syntax.

comment:8 Changed 3 years ago by sebastian

  • Blocked By 4375 added
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:9 Changed 3 years ago by sebastian

  • Blocking 4376 added

comment:10 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4047 - Improved configuration of converted JS files

comment:11 Changed 3 years ago by sebastian

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