update node to 16 and add configurable install command#45
update node to 16 and add configurable install command#45alexdor wants to merge 4 commits intopkg-size:developfrom
Conversation
|
Thanks for the PR! Can you share the use-case for customizing the installation command? Also, ideally dependency upgrades (incl Node) would be in a different PR so it can be discussed and tested in isolation but doesn't seem like a big deal here. Can you set it to Node v14 though? LTS is still active till 2023 April. |
|
Yeah, absolutely 😊 Our use case is that we are using https://rushjs.io/ for our monorepo. So the command to install the deps would be It could be autodetected, but I thought it would cover more use-cases if the users could select their install command (if they wanted to) I can also split them out in different prs if you prefer that 😊 I went for 16 because recently all the GitHub actions got bumped to 16, but yeah I'll set it to 14 😊 |
| const startTime = Date.now(); | ||
| const exitCode = await _exec(command, null, { | ||
| ...options, | ||
| silent: true, |
There was a problem hiding this comment.
So that the output of the build/install commands are visible, it makes it a lot easier when smth breaks during build or installation to see what is happening
| installCommand = autoDetectInstallCommand(); | ||
| } else { | ||
| log.info(`Custom install command providing. Installing dependencies with ${installCommand}`); | ||
| } |
There was a problem hiding this comment.
Let's move this out of the else block and log it for all cases at L22.
log.info(`Installing dependencies: ${installCommand}`);| async function npmCi({ cwd } = {}) { | ||
| async function npmCi({ cwd, installCommand } = {}) { | ||
| if (!installCommand) { | ||
| installCommand = autoDetectInstallCommand(); |
There was a problem hiding this comment.
Let's default this to npx ci: https://github.com/privatenumber/ci
There was a problem hiding this comment.
That package is not easily importable, would it be possible to export the function in a way that can be imported by other libs instead of just it being a self executed script?
| import exec from './exec.js'; | ||
|
|
||
| async function npmCi({ cwd } = {}) { | ||
| async function npmCi({ cwd, installCommand } = {}) { |
There was a problem hiding this comment.
Can you rename this function to installDependencies?
| hide-files: | ||
| description: 'Glob pattern to hide files with' | ||
| install-command: | ||
| description: 'Command to install the package with, if empty the action will try to autodetect it' |
There was a problem hiding this comment.
| description: 'Command to install the package with, if empty the action will try to autodetect it' | |
| description: 'The command used to install your project's dependencies. By default, there is autodetect logic based on `package.json#packageManager`, package lock files, etc.' |
|
Any updates on this? |
|
I need this one too because I want to use |
|
@JounQin I've been waiting for this to be merged because the current version of the package isn't usable from other libraries until @privatenumber has had some time to look through things you can use |
|
@alexdor Thanks, I have another proposal at #49, and it should be implemented at https://github.com/privatenumber/ci, I'll raise a PR for it soon. |
|
Thanks for sharing your use-cases. Using |
3 participants
No description provided.