Externaliser le code de l'analyseur #21
Conversation
…e en mémoire. Cela évite de charge le code de l'analyseur dans la page elle même. Cela permet d'analyser des sites ayant des Content Security Policy de configurés. Ces modifications sont issues de la proposition de @hayaofr (via cnumr#16), j'ai simplement ici réduit au maximum le nombre de modification pour faciliter la relecture de la PR Fix: cnumr#19
jycr
force-pushed
the
feature/19-Externaliser-le-code-de-l_analyseur
branch
from
3c345ef to
ae9b82d
Compare
| @@ -162,7 +162,7 @@ function extractBestPractices(bestPracticesFromReport) { | |||
| } | |||
|
|
|||
| function writeGlobalReport(templateEngine, globalReportVariables, outputFile) { | |||
| templateEngine.processFile('cli-core/template/global.html', globalReportVariables) | |||
| templateEngine.processFile(__dirname + '/template/global.html', globalReportVariables) | |||
There was a problem hiding this comment.
Il faudrait faire un rebase de la master : cela a été corrigé récemment
| @@ -173,7 +173,7 @@ function writeGlobalReport(templateEngine, globalReportVariables, outputFile) { | |||
|
|
|||
| function writeAllReports(templateEngine, allReportsVariables, outputFolder) { | |||
| allReportsVariables.forEach(reportVariables => { | |||
| templateEngine.processFile('cli-core/template/page.html', reportVariables) | |||
| templateEngine.processFile(__dirname + '/template/page.html', reportVariables) | |||
There was a problem hiding this comment.
Idem, en faisant un rebase de la master, cela a déjà été corrigé
| @@ -173,7 +173,7 @@ function writeGlobalReport(templateEngine, globalReportVariables, outputFile) { | |||
|
|
|||
| function writeAllReports(templateEngine, allReportsVariables, outputFolder) { | |||
| allReportsVariables.forEach(reportVariables => { | |||
| templateEngine.processFile('cli-core/template/page.html', reportVariables) | |||
| templateEngine.processFile(__dirname + '/template/page.html', reportVariables) | |||
There was a problem hiding this comment.
Idem, en faisant un rebase de la master, cela a déjà été corrigé
| @@ -16,20 +16,20 @@ | |||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| function start_analyse_core() { | |||
| function start_analyse_core(document, analyseBestPractices) { | |||
There was a problem hiding this comment.
Les modifications réalisés dans ce fichier me fait pose un peu un problème... Cela nécessite de modifier le coeur du plugin GreenIT-Analysis. En conséquence, lorsqu'on a besoin de mettre à jour le coeur de GreenIT-Analysis dans GreenIT-Analysis-cli, cela sera beaucoup plus compliqué...
Je continue à réfléchir pour voir si on a une meilleure façon de gérer la mise à jour de GreenIT-Analysis et répondre à la problématique que tu proposes de corriger.
| @@ -16,20 +16,20 @@ | |||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| function start_analyse_core() { | |||
| function start_analyse_core(document, analyseBestPractices) { | |||
There was a problem hiding this comment.
Les modifications réalisés dans ce fichier me fait pose un peu un problème... Cela nécessite de modifier le coeur du plugin GreenIT-Analysis. En conséquence, lorsqu'on a besoin de mettre à jour le coeur de GreenIT-Analysis dans GreenIT-Analysis-cli, cela sera beaucoup plus compliqué...
Je continue à réfléchir pour voir si on a une meilleure façon de gérer la mise à jour de GreenIT-Analysis et répondre à la problématique que tu proposes de corriger.
| @@ -0,0 +1,16 @@ | |||
| const {translator} = require("../cli-core/translator"); | |||
There was a problem hiding this comment.
On est en train d'inclure des dépendances du greenit-core dans cli-core et inversement... Je réfléchis aussi pour voir si on peut faire mieux
| @@ -0,0 +1,16 @@ | |||
| const {translator} = require("../cli-core/translator"); | |||
There was a problem hiding this comment.
On est en train d'inclure des dépendances du greenit-core dans cli-core et inversement... Je réfléchis aussi pour voir si on peut faire mieux
| @@ -16,17 +16,48 @@ | |||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| const utils = require('./utils.js'); | |||
There was a problem hiding this comment.
Au fur et à mesure de la lecture de la PR, je vois que par la suite, beaucoup de code faisant parti du coeur de GreenIT-Analysis sont modifiés, ce qui n'est pas le but non plus de GreenIT-Analysis-cli qui est simplement une extension. Il faut tout de même qu'on trouve une solution pour répondre à la problématique que tu remontes.
En utilisant le plugin GreenIT-Analysis, as-tu aussi des erreurs CSP ?
|
Hello @jycr, désolé de n'avoir pu regarder ta PR que maintenant. Je te laisse prendre connaissance de mes commentaires, je pense que nous avons besoin d'échanger davantage sur le problème et voir ce qu'on peut faire pour éviter au maximum de modifier le coeur de GreenIT-Analysis dans la version cli. Si tu as aussi des problèmes avec le plugin, peut-être qu'il faudrait créer une PR côté GreenIT-Analysis ? |
Changement de la méthode d'analyse en recréant le DOM de la page en mémoire.
Cela évite de charge le code de l'analyseur dans la page elle même.
Cela permet d'analyser des sites ayant des Content Security Policy de configurés.
Ces modifications sont issues de la proposition de @hayaofr (via #16), j'ai simplement ici réduit au maximum le nombre de modification pour faciliter la relecture de la PR
Fix: #19