Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

initial commit for node.js version #34

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

initial commit for node.js version #34

wants to merge 3 commits into from

Conversation

barroudjo
Copy link

Currently ruby is used to generate the pdf contracts, and given the required libraries it can only run on linux.
This adds the option to use Node.js, and work thus as well on linux and windows. Could be of use to other people than me ;-).

houndci-bot
houndci-bot reviewed Nov 27, 2018
View reviewed changes
generateContracts.js
}
}
})
.then(() => fs.unlinkSync(fileCompiledPath));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

generateContracts.js
source: fileCompiledPath,
destination: path.join(__dirname, `/contracts/${fileNameWithoutExt}.pdf`),
// debug: path.join(__dirname, `${fileNameWithoutExt}.html`), //useful if you want to check
styles: path.join(__dirname, 'md-style.css'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'__dirname' is not defined.

generateContracts.js Outdated
return mdpdf.convert({
source: fileCompiledPath,
destination: path.join(__dirname, `/contracts/${fileNameWithoutExt}.pdf`),
// debug: path.join(__dirname, `${fileNameWithoutExt}.html`), //useful if you want to check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long.

generateContracts.js
fs.writeFileSync(fileCompiledPath, result);
return mdpdf.convert({
source: fileCompiledPath,
destination: path.join(__dirname, `/contracts/${fileNameWithoutExt}.pdf`),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'__dirname' is not defined.
'template literal syntax' is only available in ES6 (use 'esversion: 6').

generateContracts.js
const fileCompiledPath = path.join(__dirname, `src/${fileNameCompiled}.md`);
const content = fs.readFileSync(path.join(__dirname, `src/${fileNameWithoutExt}.md`), 'utf8');
const template = handlebars.compile(content);
const result = template(config);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

generateContracts.js
const mdpdf = require('mdpdf');
const path = require('path');
const fs = require('fs');
const config = require('./config.json');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'require' is not defined.

generateContracts.js
const handlebars = require('handlebars')
const mdpdf = require('mdpdf');
const path = require('path');
const fs = require('fs');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'require' is not defined.

generateContracts.js
@@ -0,0 +1,41 @@
const handlebars = require('handlebars')
const mdpdf = require('mdpdf');
const path = require('path');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'require' is not defined.

generateContracts.js
@@ -0,0 +1,41 @@
const handlebars = require('handlebars')
const mdpdf = require('mdpdf');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'require' is not defined.

generateContracts.js
@@ -0,0 +1,41 @@
const handlebars = require('handlebars')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
'require' is not defined.
Missing semicolon.

@JalilArfaoui
Copy link

Maybe you could add yarn.lock or package-lock.json ?

@JalilArfaoui
Copy link

(And thanks for this, first of all :-) )

@barroudjo
Copy link
Author

You're right, I forgot to do this. Thanks for the heads-up, and thanks for the thanks !
BTW maybe I should update the readme as well.

houndci-bot
houndci-bot reviewed Nov 28, 2018
View reviewed changes
generateContracts.js
return mdpdf.convert({
source: fileCompiledPath,
destination: path.join(__dirname, `/contracts/${fileNameWithoutExt}.pdf`),
debug: args.debug && path.join(__dirname, `/contracts/${fileNameWithoutExt}.html`),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'__dirname' is not defined.
'template literal syntax' is only available in ES6 (use 'esversion: 6').
Line is too long.

generateContracts.js
const fs = require('fs');
const config = require('./config.json');
const args = {
debug: process.argv.includes('--debug')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'process' is not defined.

generateContracts.js
const path = require('path');
const fs = require('fs');
const config = require('./config.json');
const args = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

@barroudjo
Copy link
Author

barroudjo commented Dec 6, 2018 •
edited
Loading

@tibastral is there something preventing this PR from being merged ? I can take care of it if there is ;-)

All the issues that houndci-bot shows are not issues actually, they're just due to its configuration, which should be updated. I'm taking a look at it.

EDIT: there's no houndci-bot configuration files so I don't see how I can configure it.

@tibastral
Copy link
Owner

Tu peux ajouter justement ce fichier de conf ?

Petite question : pourquoi ne pas supprimer cette dépendance qui fait que ça marche pas avec ruby plutôt que de tout refaire en node ?

@barroudjo
Copy link
Author

Je vais vérifier, mais la dépendance c'est libv8, et elle me semble cruciale (plusieurs des libs de base du gemfile en ont besoin si je ne m'abuse).
L'autre intérêt c'est que node.js est plus petit à installer, et que ça pourra peut être servir à d'autres d'avoir cette alternative dans un autre langage (pour customiser l'output par exemple).

@tibastral
Copy link
Owner

tibastral commented Dec 6, 2018 via email

@barroudjo
Copy link
Author

barroudjo commented Dec 6, 2018 via email

@tibastral
Copy link
Owner

tibastral commented Dec 6, 2018 via email

@barroudjo
Copy link
Author

barroudjo commented Dec 6, 2018 via email

@tibastral
Copy link
Owner

tibastral commented Dec 6, 2018 via email

@barroudjo
Copy link
Author

barroudjo commented Dec 7, 2018 via email

@tibastral
Copy link
Owner

tibastral commented Dec 7, 2018 via email

@barroudjo
Copy link
Author

barroudjo commented Dec 7, 2018 via email

@tibastral
Copy link
Owner

EN fait pour tout dire, on avait migré vers JS il y a quelques temps, mais ensuite, on a re-migré vers ruby notamment parce que j'avais pas envie de maintenir du js. Après, si tu es chaud pour faire une version elm, go for it, je mergerai avec joie, mais js je suis vraiment pas fan.

houndci-bot
houndci-bot reviewed Oct 18, 2021
View reviewed changes
generateContracts.js

generatePdf('cgv')
.then(() => generatePdf('cp'))
.then(() => generatePdf('cga'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

generateContracts.js
}

generatePdf('cgv')
.then(() => generatePdf('cp'))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@barroudjo @JalilArfaoui @tibastral @houndci-bot