chore: @talend/scripts module #1098
Conversation
1 similar comment
packages/scripts/scripts/utils.js
Outdated
| ' | \'--------------\' || \'--------------\' || \'--------------\' || \'--------------\' || \'--------------\' || \'--------------\' || \'--------------\' |\n' + | ||
| ' \'----------------\' \'----------------\' \'----------------\' \'----------------\' \'----------------\' \'----------------\' \'----------------\' \n' + | ||
| '###################################################################################################################################################################'); | ||
| } |
There was a problem hiding this comment.
Remove that useless ascii art.
packages/scripts/scripts/utils.js
Outdated
| try { | ||
| pathFromWhich = fs.realpathSync(which.sync(executable)); | ||
| } catch (_error) { | ||
| // ignore _error |
There was a problem hiding this comment.
Remove that try/catch if you don't intend of using the error.
There was a problem hiding this comment.
No it's to not throw the error from which module call. If the command fails, the script should not fail.
packages/scripts/scripts/utils.js
Outdated
| * @returns {*} The executable path | ||
| */ | ||
| function resolveBin(modName, { executable = modName, cwd = process.cwd() } = {}) { | ||
| let pathFromWhich; |
There was a problem hiding this comment.
Rename pathFromWhich to something meaningful.
packages/scripts/scripts/utils.js
Outdated
| @@ -0,0 +1,175 @@ | |||
| /* eslint-disable global-require */ | |||
There was a problem hiding this comment.
Please properly rename that script into something that describes its content.
packages/scripts/scripts/utils.js
Outdated
| */ | ||
| function hereRelative(dirname, p) { | ||
| return path | ||
| .join(dirname, p) |
There was a problem hiding this comment.
path is already used by nodeJs' path
There was a problem hiding this comment.
I'll find something
packages/scripts/scripts/utils.js
Outdated
| * Create a user configuration getter | ||
| * @returns {getUserConfig} The user configuration getter | ||
| */ | ||
| function createUserConfigGetter() { |
There was a problem hiding this comment.
I don't understand why you need a getter creator. Can't you just do the call from inside the getter?
There was a problem hiding this comment.
It's to memoize the talendScriptsConfig.
It is transported in environment var, so it is serialized (JSON.stringify). To get it, I parse it (JSON.parse). This allows to parse it 1 time and then each call to get() just use it.
packages/scripts/README.md
Outdated
| ### Default usage | ||
|
|
||
| 1. Add @talend/scripts as dev dependency. | ||
| ``` |
There was a problem hiding this comment.
You can use shell or bash here
1 similar comment
What is the problem this PR is trying to solve?
Each app has it's build/test/lint configuration, even if they are the same or almost the same.
What is the chosen solution to this problem?
Create a module
@talend/scriptsthat exposes bins to run common scripts with common configuration. Those configurations can be overridden or enhanced by app custom configuration.This PR is only on build (webpack). Please refer to the README to understand how it works.
It is still on webpack 3 because there are plugins and loaders that are not ready yet :
Please check if the PR fulfills these requirements
[ ] This PR introduces a breaking change