public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 0/3] make linting threaded
@ 2021-07-19 10:31 Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw)
  To: pve-devel

NOTE: this series will not build until a 'make buildupstream' is
executed (and 'src/lib/eslint.js' should be committed as well). I
did not send it because it would be too big.

this series convert the package into a proper nodejs module
'pve-eslint', and adds threading to the linting binary

changes from v1:
* convert to nodejs module
* split worker code to own file
* drop hacky self-loading script, and use the 'pve-eslint' module instead

Dominik Csapak (3):
  ship proper nodejs module 'pve-eslint'
  remove unnecessary eslint.js
  use worker_threads for linting

 Makefile                                |      2 +-
 debian/control                          |      7 +-
 debian/dirs                             |      1 +
 debian/links                            |      1 +
 debian/rules                            |      5 +-
 patches/0001-adapt-webpack-config.patch |     19 +-
 src/Makefile                            |     15 -
 src/{ => bin}/app.js                    |     40 +-
 src/eslint.js                           | 138871 ---------------------
 src/index.js                            |      5 +
 src/lib/worker.js                       |     27 +
 src/package.json                        |     10 +
 12 files changed, 92 insertions(+), 138911 deletions(-)
 create mode 100644 debian/dirs
 create mode 100644 debian/links
 delete mode 100644 src/Makefile
 rename src/{ => bin}/app.js (92%)
 delete mode 100755 src/eslint.js
 create mode 100644 src/index.js
 create mode 100644 src/lib/worker.js
 create mode 100644 src/package.json

-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak
@ 2021-07-19 10:31 ` Dominik Csapak
  2021-08-25 16:38   ` Thomas Lamprecht
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak
  2 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw)
  To: pve-devel

instead of concatenating the eslint module into our app.js, ship
a 'pve-eslint' module that exports the built eslint module

to do this, we have to leave the module type on 'umd' instead of
changing to 'var' so that nodejs can properly import it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 Makefile                                |  2 +-
 debian/control                          |  7 +++++--
 debian/dirs                             |  1 +
 debian/links                            |  1 +
 debian/rules                            |  5 ++++-
 patches/0001-adapt-webpack-config.patch | 19 +++++--------------
 src/Makefile                            | 15 ---------------
 src/{ => bin}/app.js                    |  5 ++++-
 src/index.js                            |  3 +++
 src/package.json                        |  9 +++++++++
 10 files changed, 33 insertions(+), 34 deletions(-)
 create mode 100644 debian/dirs
 create mode 100644 debian/links
 delete mode 100644 src/Makefile
 rename src/{ => bin}/app.js (99%)
 create mode 100644 src/index.js
 create mode 100644 src/package.json

diff --git a/Makefile b/Makefile
index 9dbe3d0..2a6666c 100644
--- a/Makefile
+++ b/Makefile
@@ -49,7 +49,7 @@ download:
 # NOTE: needs npm installed, downloads packages from npm
 .PHONY: buildupstream
 buildupstream: ${BUILDSRC}
-	cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/eslint.js
+	cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/lib/eslint.js
 
 ${BUILDSRC}: ${UPSTREAM} patches
 	rm -rf $@
diff --git a/debian/control b/debian/control
index 3f9b014..7ea3664 100644
--- a/debian/control
+++ b/debian/control
@@ -2,13 +2,16 @@ Source: pve-eslint
 Section: devel
 Priority: optional
 Maintainer: Proxmox Support Team <support@proxmox.com>
-Build-Depends: debhelper (>= 12~)
+Build-Depends: debhelper (>= 12~),
+               nodejs,
+               pkg-js-tools (>= 0.8.11)
 Standards-Version: 4.3.0
 Homepage: http://www.proxmox.com
 
 Package: pve-eslint
 Architecture: all
-Depends: node-commander, node-colors, nodejs, ${misc:Depends},
+Depends: node-commander, node-colors, nodejs (>= ${nodejs:Version}), ${misc:Depends},
+Provides: ${nodejs:Provides}
 Description: ESLint for Proxmox Virtual Environment development
  This package contains a version of eslint used to develop the
  Proxmox Virtual Environment, and other Proxmox projects, web GUI.
diff --git a/debian/dirs b/debian/dirs
new file mode 100644
index 0000000..e772481
--- /dev/null
+++ b/debian/dirs
@@ -0,0 +1 @@
+usr/bin
diff --git a/debian/links b/debian/links
new file mode 100644
index 0000000..99342ed
--- /dev/null
+++ b/debian/links
@@ -0,0 +1 @@
+usr/share/nodejs/pve-eslint/bin/app.js usr/bin/eslint
diff --git a/debian/rules b/debian/rules
index 2d33f6a..b4c4090 100755
--- a/debian/rules
+++ b/debian/rules
@@ -1,4 +1,7 @@
 #!/usr/bin/make -f
 
 %:
-	dh $@
+	dh $@ --with nodejs
+
+execute_after_dh_fixperms:
+	chmod --recursive a+x -- debian/*/usr/share/nodejs/pve-eslint/bin/*
diff --git a/patches/0001-adapt-webpack-config.patch b/patches/0001-adapt-webpack-config.patch
index 4698e74..b0201e1 100644
--- a/patches/0001-adapt-webpack-config.patch
+++ b/patches/0001-adapt-webpack-config.patch
@@ -3,21 +3,20 @@ From: Dominik Csapak <d.csapak@proxmox.com>
 Date: Thu, 2 Apr 2020 07:10:18 +0000
 Subject: [PATCH] adapt webpack config
 
-changes to 'var' from 'umd' since we want to use it in the same file
 adds 'cli-engine' to build (we use it in our wrapper)
 and target 'node' since we will use it on the cli
 
 Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
 ---
- webpack.config.js | 6 +++---
- 1 file changed, 3 insertions(+), 3 deletions(-)
+ webpack.config.js | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/webpack.config.js b/webpack.config.js
-index 29d60cb4..95027075 100644
+index a22c99b..9209159 100644
 --- a/webpack.config.js
 +++ b/webpack.config.js
-@@ -2,14 +2,14 @@
- 
+@@ -4,8 +4,9 @@ const NodePolyfillPlugin = require("node-polyfill-webpack-plugin");
+ /** @type {import("webpack").Configuration} */
  module.exports = {
      mode: "none",
 +    target: "node",
@@ -27,13 +26,5 @@ index 29d60cb4..95027075 100644
      },
      output: {
          filename: "[name].js",
-         library: "[name]",
--        libraryTarget: "umd",
--        globalObject: "this"
-+        libraryTarget: "var"
-     },
-     module: {
-         rules: [
 -- 
 2.20.1
-
diff --git a/src/Makefile b/src/Makefile
deleted file mode 100644
index bef1c57..0000000
diff --git a/src/app.js b/src/bin/app.js
similarity index 99%
rename from src/app.js
rename to src/bin/app.js
index 9226234..8a28923 100644
--- a/src/app.js
+++ b/src/bin/app.js
@@ -1,9 +1,12 @@
-(function() {
+#!/usr/bin/env node
+
+(async function(){
 'use strict';
 
 const path = require('path');
 const color = require('colors');
 const program = require('commander');
+const eslint = require('pve-eslint');
 
 program
     .usage('[options] [<file(s) ...>]')
diff --git a/src/index.js b/src/index.js
new file mode 100644
index 0000000..01b9a1d
--- /dev/null
+++ b/src/index.js
@@ -0,0 +1,3 @@
+const eslint = require('./lib/eslint.js');
+
+module.exports = eslint;
diff --git a/src/package.json b/src/package.json
new file mode 100644
index 0000000..b08184b
--- /dev/null
+++ b/src/package.json
@@ -0,0 +1,9 @@
+{
+    "name": "pve-eslint",
+    "version": "7.28.0",
+    "files": [
+	"index.js",
+	"bin/app.js",
+	"lib/eslint.js"
+    ]
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js
  2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
@ 2021-07-19 10:31 ` Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak
  2 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/eslint.js | 138871 -----------------------------------------------
 1 file changed, 138871 deletions(-)
 delete mode 100755 src/eslint.js

diff --git a/src/eslint.js b/src/eslint.js
deleted file mode 100755
index 6115dea..0000000
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pve-devel] [PATCH v2 3/3] use worker_threads for linting
  2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak
@ 2021-07-19 10:31 ` Dominik Csapak
  2 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw)
  To: pve-devel

instead linting all files in the main thread, use worker threads
for that (4 by default) and add the '-t' switch to able to control that

a basic benchmark of eslint of pve-manager showed some performance
gains:

Benchmark #1: Current
  Time (mean ± σ):      6.468 s ±  0.116 s    [User: 9.803 s, System: 0.333 s]
  Range (min … max):    6.264 s …  6.647 s    10 runs

Benchmark #2: 2Threads
  Time (mean ± σ):      4.509 s ±  0.106 s    [User: 12.706 s, System: 0.530 s]
  Range (min … max):    4.335 s …  4.674 s    10 runs

Benchmark #3: 4Threads
  Time (mean ± σ):      3.471 s ±  0.033 s    [User: 16.390 s, System: 0.630 s]
  Range (min … max):    3.431 s …  3.542 s    10 runs

Benchmark #4: 8Threads
  Time (mean ± σ):      2.880 s ±  0.044 s    [User: 22.454 s, System: 0.938 s]
  Range (min … max):    2.813 s …  2.964 s    10 runs

Summary
  '8Threads' ran
    1.21 ± 0.02 times faster than '4Threads'
    1.57 ± 0.04 times faster than '2Threads'
    2.25 ± 0.05 times faster than 'Current'

after 8 threads, there were no real performance benefits since the
overhead to load the module seems to be the biggest factor.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/app.js    | 35 +++++++++++++++++++++++++++++------
 src/index.js      |  2 ++
 src/lib/worker.js | 27 +++++++++++++++++++++++++++
 src/package.json  |  3 ++-
 4 files changed, 60 insertions(+), 7 deletions(-)
 create mode 100644 src/lib/worker.js

diff --git a/src/bin/app.js b/src/bin/app.js
index 8a28923..10e7e6a 100644
--- a/src/bin/app.js
+++ b/src/bin/app.js
@@ -6,6 +6,7 @@
 const path = require('path');
 const color = require('colors');
 const program = require('commander');
+const worker = require('worker_threads');
 const eslint = require('pve-eslint');
 
 program
@@ -14,6 +15,7 @@ program
     .option('-e, --extend <configfile>', 'uses <configfile> ontop of default eslint config.')
     .option('-f, --fix', 'if set, fixes will be applied.')
     .option('-s, --strict', 'if set, also exit uncleanly on warnings')
+    .option('-t, --threads <threads>', 'how many worker_threads should be used (default=4)')
     .option('--output-config', 'if set, only output the config as JSON and exit.')
     ;
 
@@ -42,6 +44,11 @@ if (!paths.length) {
     paths = [process.cwd()];
 }
 
+let threadCount = 4;
+if (program.threads) {
+    threadCount = program.threads;
+}
+
 const defaultConfig = {
     parserOptions: {
 	ecmaVersion: 2020,
@@ -283,20 +290,36 @@ if (program.outputConfig) {
     process.exit(0);
 }
 
-const cli = new eslint.CLIEngine({
+const cliOptions = {
     baseConfig: config,
     useEslintrc: true,
     fix: !!program.fix,
     cwd: process.cwd(),
-});
+};
+
+let promises = [];
+let filesPerThread = Math.round(paths.length / threadCount);
+for (let i = 0; i < (threadCount - 1); i++) {
+    let files = paths.splice(0, filesPerThread);
+    promises.push(eslint.createWorker({
+	cliOptions,
+	files
+    }));
+}
+
+// the remaining paths
+promises.push(eslint.createWorker({
+    cliOptions,
+    files: paths
+}));
 
-const report = cli.executeOnFiles(paths);
+let results = (await Promise.all(promises)).map(res => res.results).flat(1);
 
 let exitcode = 0;
 let files_err = [], files_warn = [], files_ok = [];
 let fixes = 0;
 console.log('------------------------------------------------------------');
-report.results.forEach(function(result) {
+results.forEach(function(result) {
     let filename = path.relative(process.cwd(), result.filePath);
     let msgs = result.messages;
     let max_sev = 0;
@@ -348,7 +371,7 @@ report.results.forEach(function(result) {
     console.log('------------------------------------------------------------');
 });
 
-if (report.results.length > 1) {
+if (results.length > 1) {
     console.log(`${color.bold(files_ok.length + files_err.length)} files:`);
     if (files_err.length > 0) {
 	console.log(color.red(` ${color.bold(files_err.length)} files have Errors`));
@@ -367,7 +390,7 @@ console.log('------------------------------------------------------------');
 if (program.fix) {
     if (fixes > 0) {
 	console.log(`Writing ${color.bold(fixes)} fixed files...`);
-	eslint.CLIEngine.outputFixes(report);
+	eslint.CLIEngine.outputFixes({ results });
 	console.log('Done');
     } else {
 	console.log("No fixable Errors/Warnings found.");
diff --git a/src/index.js b/src/index.js
index 01b9a1d..311ae38 100644
--- a/src/index.js
+++ b/src/index.js
@@ -1,3 +1,5 @@
 const eslint = require('./lib/eslint.js');
+const createWorker = require('./lib/worker.js');
 
 module.exports = eslint;
+module.exports.createWorker = createWorker;
diff --git a/src/lib/worker.js b/src/lib/worker.js
new file mode 100644
index 0000000..9a8c955
--- /dev/null
+++ b/src/lib/worker.js
@@ -0,0 +1,27 @@
+'use strict';
+
+const worker = require('worker_threads');
+
+if (!worker.isMainThread) {
+    const eslint = require('pve-eslint');
+    const data = worker.workerData;
+    const cli = new eslint.CLIEngine(data.cliOptions);
+    const report = cli.executeOnFiles(data.files);
+    worker.parentPort.postMessage(report);
+} else {
+    module.exports = async function createWorker(workerData) {
+	return new Promise((resolve, reject) => {
+	    const child = new worker.Worker(__filename,
+		{
+		    workerData,
+		},
+	    );
+	    child.on('message', resolve);
+	    child.on('error', reject);
+	    child.on('exit', (code) => {
+		if (code !== 0) {reject(new Error(`Worker stopped with exit code ${code}`));}
+	    });
+	});
+    }
+}
+
diff --git a/src/package.json b/src/package.json
index b08184b..e912069 100644
--- a/src/package.json
+++ b/src/package.json
@@ -4,6 +4,7 @@
     "files": [
 	"index.js",
 	"bin/app.js",
-	"lib/eslint.js"
+	"lib/eslint.js",
+	"lib/worker.js"
     ]
 }
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
@ 2021-08-25 16:38   ` Thomas Lamprecht
  2021-08-30  9:17     ` Dominik Csapak
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Lamprecht @ 2021-08-25 16:38 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak

On 19/07/2021 12:31, Dominik Csapak wrote:
> instead of concatenating the eslint module into our app.js, ship
> a 'pve-eslint' module that exports the built eslint module
> 
> to do this, we have to leave the module type on 'umd' instead of
> changing to 'var' so that nodejs can properly import it.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  Makefile                                |  2 +-

Does not applies here, did not really investigated yet though:

Applying: ship proper nodejs module 'pve-eslint'
Using index info to reconstruct a base tree...               
error: removal patch leaves file contents
error: src/Makefile: patch does not apply


>  debian/control                          |  7 +++++--
>  debian/dirs                             |  1 +
>  debian/links                            |  1 +
>  debian/rules                            |  5 ++++-
>  patches/0001-adapt-webpack-config.patch | 19 +++++--------------
>  src/Makefile                            | 15 ---------------
>  src/{ => bin}/app.js                    |  5 ++++-
>  src/index.js                            |  3 +++
>  src/package.json                        |  9 +++++++++
>  10 files changed, 33 insertions(+), 34 deletions(-)
>  create mode 100644 debian/dirs
>  create mode 100644 debian/links
>  delete mode 100644 src/Makefile
>  rename src/{ => bin}/app.js (99%)
>  create mode 100644 src/index.js
>  create mode 100644 src/package.json
> 
> diff --git a/Makefile b/Makefile
> index 9dbe3d0..2a6666c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -49,7 +49,7 @@ download:
>  # NOTE: needs npm installed, downloads packages from npm
>  .PHONY: buildupstream
>  buildupstream: ${BUILDSRC}
> -	cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/eslint.js
> +	cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/lib/eslint.js
>  
>  ${BUILDSRC}: ${UPSTREAM} patches
>  	rm -rf $@
> diff --git a/debian/control b/debian/control
> index 3f9b014..7ea3664 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -2,13 +2,16 @@ Source: pve-eslint
>  Section: devel
>  Priority: optional
>  Maintainer: Proxmox Support Team <support@proxmox.com>
> -Build-Depends: debhelper (>= 12~)
> +Build-Depends: debhelper (>= 12~),
> +               nodejs,
> +               pkg-js-tools (>= 0.8.11)
>  Standards-Version: 4.3.0
>  Homepage: http://www.proxmox.com
>  
>  Package: pve-eslint
>  Architecture: all
> -Depends: node-commander, node-colors, nodejs, ${misc:Depends},
> +Depends: node-commander, node-colors, nodejs (>= ${nodejs:Version}), ${misc:Depends},
> +Provides: ${nodejs:Provides}
>  Description: ESLint for Proxmox Virtual Environment development
>   This package contains a version of eslint used to develop the
>   Proxmox Virtual Environment, and other Proxmox projects, web GUI.
> diff --git a/debian/dirs b/debian/dirs
> new file mode 100644
> index 0000000..e772481
> --- /dev/null
> +++ b/debian/dirs
> @@ -0,0 +1 @@
> +usr/bin
> diff --git a/debian/links b/debian/links
> new file mode 100644
> index 0000000..99342ed
> --- /dev/null
> +++ b/debian/links
> @@ -0,0 +1 @@
> +usr/share/nodejs/pve-eslint/bin/app.js usr/bin/eslint
> diff --git a/debian/rules b/debian/rules
> index 2d33f6a..b4c4090 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -1,4 +1,7 @@
>  #!/usr/bin/make -f
>  
>  %:
> -	dh $@
> +	dh $@ --with nodejs
> +
> +execute_after_dh_fixperms:
> +	chmod --recursive a+x -- debian/*/usr/share/nodejs/pve-eslint/bin/*
> diff --git a/patches/0001-adapt-webpack-config.patch b/patches/0001-adapt-webpack-config.patch
> index 4698e74..b0201e1 100644
> --- a/patches/0001-adapt-webpack-config.patch
> +++ b/patches/0001-adapt-webpack-config.patch
> @@ -3,21 +3,20 @@ From: Dominik Csapak <d.csapak@proxmox.com>
>  Date: Thu, 2 Apr 2020 07:10:18 +0000
>  Subject: [PATCH] adapt webpack config
>  
> -changes to 'var' from 'umd' since we want to use it in the same file
>  adds 'cli-engine' to build (we use it in our wrapper)
>  and target 'node' since we will use it on the cli
>  
>  Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>  ---
> - webpack.config.js | 6 +++---
> - 1 file changed, 3 insertions(+), 3 deletions(-)
> + webpack.config.js | 3 ++-
> + 1 file changed, 2 insertions(+), 1 deletion(-)
>  
>  diff --git a/webpack.config.js b/webpack.config.js
> -index 29d60cb4..95027075 100644
> +index a22c99b..9209159 100644
>  --- a/webpack.config.js
>  +++ b/webpack.config.js
> -@@ -2,14 +2,14 @@
> - 
> +@@ -4,8 +4,9 @@ const NodePolyfillPlugin = require("node-polyfill-webpack-plugin");
> + /** @type {import("webpack").Configuration} */
>   module.exports = {
>       mode: "none",
>  +    target: "node",
> @@ -27,13 +26,5 @@ index 29d60cb4..95027075 100644
>       },
>       output: {
>           filename: "[name].js",
> -         library: "[name]",
> --        libraryTarget: "umd",
> --        globalObject: "this"
> -+        libraryTarget: "var"
> -     },
> -     module: {
> -         rules: [
>  -- 
>  2.20.1
> -
> diff --git a/src/Makefile b/src/Makefile
> deleted file mode 100644
> index bef1c57..0000000
> diff --git a/src/app.js b/src/bin/app.js
> similarity index 99%
> rename from src/app.js
> rename to src/bin/app.js
> index 9226234..8a28923 100644
> --- a/src/app.js
> +++ b/src/bin/app.js
> @@ -1,9 +1,12 @@
> -(function() {
> +#!/usr/bin/env node
> +
> +(async function(){
>  'use strict';
>  
>  const path = require('path');
>  const color = require('colors');
>  const program = require('commander');
> +const eslint = require('pve-eslint');
>  
>  program
>      .usage('[options] [<file(s) ...>]')
> diff --git a/src/index.js b/src/index.js
> new file mode 100644
> index 0000000..01b9a1d
> --- /dev/null
> +++ b/src/index.js
> @@ -0,0 +1,3 @@
> +const eslint = require('./lib/eslint.js');
> +
> +module.exports = eslint;
> diff --git a/src/package.json b/src/package.json
> new file mode 100644
> index 0000000..b08184b
> --- /dev/null
> +++ b/src/package.json
> @@ -0,0 +1,9 @@
> +{
> +    "name": "pve-eslint",
> +    "version": "7.28.0",
> +    "files": [
> +	"index.js",
> +	"bin/app.js",
> +	"lib/eslint.js"
> +    ]
> +}
> 





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-25 16:38   ` Thomas Lamprecht
@ 2021-08-30  9:17     ` Dominik Csapak
  2021-08-30  9:25       ` Thomas Lamprecht
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2021-08-30  9:17 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

On 8/25/21 18:38, Thomas Lamprecht wrote:
> On 19/07/2021 12:31, Dominik Csapak wrote:
>> instead of concatenating the eslint module into our app.js, ship
>> a 'pve-eslint' module that exports the built eslint module
>>
>> to do this, we have to leave the module type on 'umd' instead of
>> changing to 'var' so that nodejs can properly import it.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   Makefile                                |  2 +-
> 
> Does not applies here, did not really investigated yet though:
> 
> Applying: ship proper nodejs module 'pve-eslint'
> Using index info to reconstruct a base tree...
> error: removal patch leaves file contents
> error: src/Makefile: patch does not apply
> 
> 

ah yes, sorry, i created the patches with '-D'
since the second patch would not fit on the mailing list

shall i send 1/3 and 3/3 again with out -D ?
the only conflict is that src/Makefile is deleted
(2/3 can be done separately anyway)





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-30  9:17     ` Dominik Csapak
@ 2021-08-30  9:25       ` Thomas Lamprecht
  2021-08-30 10:06         ` Dominik Csapak
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Lamprecht @ 2021-08-30  9:25 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion

Am 8/30/21 um 11:17 AM schrieb Dominik Csapak:
> On 8/25/21 18:38, Thomas Lamprecht wrote:
>> On 19/07/2021 12:31, Dominik Csapak wrote:
>>> instead of concatenating the eslint module into our app.js, ship
>>> a 'pve-eslint' module that exports the built eslint module
>>>
>>> to do this, we have to leave the module type on 'umd' instead of
>>> changing to 'var' so that nodejs can properly import it.
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>>   Makefile                                |  2 +-
>>
>> Does not applies here, did not really investigated yet though:
>>
>> Applying: ship proper nodejs module 'pve-eslint'
>> Using index info to reconstruct a base tree...
>> error: removal patch leaves file contents
>> error: src/Makefile: patch does not apply
>>
>>
> 
> ah yes, sorry, i created the patches with '-D'
> since the second patch would not fit on the mailing list
> 
> shall i send 1/3 and 3/3 again with out -D ?
> the only conflict is that src/Makefile is deleted
> (2/3 can be done separately anyway)
> 
> 

I can also pull this from a staff repo if you prefer that?




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-30  9:25       ` Thomas Lamprecht
@ 2021-08-30 10:06         ` Dominik Csapak
  2021-08-30 10:36           ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2021-08-30 10:06 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

On 8/30/21 11:25, Thomas Lamprecht wrote:
> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak:
>> On 8/25/21 18:38, Thomas Lamprecht wrote:
>>> On 19/07/2021 12:31, Dominik Csapak wrote:
>>>> instead of concatenating the eslint module into our app.js, ship
>>>> a 'pve-eslint' module that exports the built eslint module
>>>>
>>>> to do this, we have to leave the module type on 'umd' instead of
>>>> changing to 'var' so that nodejs can properly import it.
>>>>
>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>>> ---
>>>>    Makefile                                |  2 +-
>>>
>>> Does not applies here, did not really investigated yet though:
>>>
>>> Applying: ship proper nodejs module 'pve-eslint'
>>> Using index info to reconstruct a base tree...
>>> error: removal patch leaves file contents
>>> error: src/Makefile: patch does not apply
>>>
>>>
>>
>> ah yes, sorry, i created the patches with '-D'
>> since the second patch would not fit on the mailing list
>>
>> shall i send 1/3 and 3/3 again with out -D ?
>> the only conflict is that src/Makefile is deleted
>> (2/3 can be done separately anyway)
>>
>>
> 
> I can also pull this from a staff repo if you prefer that?
> 

ok, its the 'nodejs' branch of my staff repo for 'pve-eslint'




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-30 10:06         ` Dominik Csapak
@ 2021-08-30 10:36           ` Thomas Lamprecht
  2021-08-30 10:43             ` Dominik Csapak
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Lamprecht @ 2021-08-30 10:36 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak

Am 8/30/21 um 12:06 PM schrieb Dominik Csapak:
> On 8/30/21 11:25, Thomas Lamprecht wrote:
>> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak:
>>> On 8/25/21 18:38, Thomas Lamprecht wrote:
>>>> On 19/07/2021 12:31, Dominik Csapak wrote:
>>>>> instead of concatenating the eslint module into our app.js, ship
>>>>> a 'pve-eslint' module that exports the built eslint module
>>>>>
>>>>> to do this, we have to leave the module type on 'umd' instead of
>>>>> changing to 'var' so that nodejs can properly import it.
>>>>>
>>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>>>> ---
>>>>>    Makefile                                |  2 +-
>>>>
>>>> Does not applies here, did not really investigated yet though:
>>>>
>>>> Applying: ship proper nodejs module 'pve-eslint'
>>>> Using index info to reconstruct a base tree...
>>>> error: removal patch leaves file contents
>>>> error: src/Makefile: patch does not apply
>>>>
>>>>
>>>
>>> ah yes, sorry, i created the patches with '-D'
>>> since the second patch would not fit on the mailing list
>>>
>>> shall i send 1/3 and 3/3 again with out -D ?
>>> the only conflict is that src/Makefile is deleted
>>> (2/3 can be done separately anyway)
>>>
>>>
>>
>> I can also pull this from a staff repo if you prefer that?
>>
> 
> ok, its the 'nodejs' branch of my staff repo for 'pve-eslint'
> 

great, applied, thanks!




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-30 10:36           ` [pve-devel] applied: " Thomas Lamprecht
@ 2021-08-30 10:43             ` Dominik Csapak
  2021-08-30 11:03               ` Thomas Lamprecht
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2021-08-30 10:43 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

On 8/30/21 12:36, Thomas Lamprecht wrote:
> Am 8/30/21 um 12:06 PM schrieb Dominik Csapak:
>> On 8/30/21 11:25, Thomas Lamprecht wrote:
>>> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak:
>>>> On 8/25/21 18:38, Thomas Lamprecht wrote:
>>>>> On 19/07/2021 12:31, Dominik Csapak wrote:
>>>>>> instead of concatenating the eslint module into our app.js, ship
>>>>>> a 'pve-eslint' module that exports the built eslint module
>>>>>>
>>>>>> to do this, we have to leave the module type on 'umd' instead of
>>>>>> changing to 'var' so that nodejs can properly import it.
>>>>>>
>>>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>>>>> ---
>>>>>>     Makefile                                |  2 +-
>>>>>
>>>>> Does not applies here, did not really investigated yet though:
>>>>>
>>>>> Applying: ship proper nodejs module 'pve-eslint'
>>>>> Using index info to reconstruct a base tree...
>>>>> error: removal patch leaves file contents
>>>>> error: src/Makefile: patch does not apply
>>>>>
>>>>>
>>>>
>>>> ah yes, sorry, i created the patches with '-D'
>>>> since the second patch would not fit on the mailing list
>>>>
>>>> shall i send 1/3 and 3/3 again with out -D ?
>>>> the only conflict is that src/Makefile is deleted
>>>> (2/3 can be done separately anyway)
>>>>
>>>>
>>>
>>> I can also pull this from a staff repo if you prefer that?
>>>
>>
>> ok, its the 'nodejs' branch of my staff repo for 'pve-eslint'
>>
> 
> great, applied, thanks!
> 

thx, please do not forget to run 'make buildupstream' and
commiting the eslint file before bumping :)
(or i can do it and push to my staff repo if you want)




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint'
  2021-08-30 10:43             ` Dominik Csapak
@ 2021-08-30 11:03               ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-08-30 11:03 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion

Am 8/30/21 um 12:43 PM schrieb Dominik Csapak:
> On 8/30/21 12:36, Thomas Lamprecht wrote:
>> Am 8/30/21 um 12:06 PM schrieb Dominik Csapak:
>>> On 8/30/21 11:25, Thomas Lamprecht wrote:
>>>> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak:
>>>>> On 8/25/21 18:38, Thomas Lamprecht wrote:
>>>>>> On 19/07/2021 12:31, Dominik Csapak wrote:
>>>>>>> instead of concatenating the eslint module into our app.js, ship
>>>>>>> a 'pve-eslint' module that exports the built eslint module
>>>>>>>
>>>>>>> to do this, we have to leave the module type on 'umd' instead of
>>>>>>> changing to 'var' so that nodejs can properly import it.
>>>>>>>
>>>>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>>>>>> ---
>>>>>>>     Makefile                                |  2 +-
>>>>>>
>>>>>> Does not applies here, did not really investigated yet though:
>>>>>>
>>>>>> Applying: ship proper nodejs module 'pve-eslint'
>>>>>> Using index info to reconstruct a base tree...
>>>>>> error: removal patch leaves file contents
>>>>>> error: src/Makefile: patch does not apply
>>>>>>
>>>>>>
>>>>>
>>>>> ah yes, sorry, i created the patches with '-D'
>>>>> since the second patch would not fit on the mailing list
>>>>>
>>>>> shall i send 1/3 and 3/3 again with out -D ?
>>>>> the only conflict is that src/Makefile is deleted
>>>>> (2/3 can be done separately anyway)
>>>>>
>>>>>
>>>>
>>>> I can also pull this from a staff repo if you prefer that?
>>>>
>>>
>>> ok, its the 'nodejs' branch of my staff repo for 'pve-eslint'
>>>
>>
>> great, applied, thanks!
>>
> 
> thx, please do not forget to run 'make buildupstream' and
> commiting the eslint file before bumping :)
> (or i can do it and push to my staff repo if you want)
> 

Without that it did not build anyway (I wanted to test it), but thanks
for the reminder - updated+committed build got pushed out now.




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-30 11:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak
2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
2021-08-25 16:38   ` Thomas Lamprecht
2021-08-30  9:17     ` Dominik Csapak
2021-08-30  9:25       ` Thomas Lamprecht
2021-08-30 10:06         ` Dominik Csapak
2021-08-30 10:36           ` [pve-devel] applied: " Thomas Lamprecht
2021-08-30 10:43             ` Dominik Csapak
2021-08-30 11:03               ` Thomas Lamprecht
2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak
2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal