WIP: Create separate commits per upgraded package #39
Conversation
TODO: Detect if yarn or npm used, also detect if lock file exists and should be included
This reverts commit 20d34de.
| @@ -18,6 +18,7 @@ import askUser from '../askUser'; | |||
| import {toSentence} from '../stringUtils'; | |||
| import {askIgnoreFields} from './ignore'; | |||
| import Config from '../Config'; | |||
| import { execSync } from 'child_process'; | |||
There was a problem hiding this comment.
Let's move it below import {writeFileSync, existsSync} from 'fs'
| @@ -154,6 +155,7 @@ export const handler = catchAsyncError(async opts => { | |||
| `from ${from} to ${colorizeDiff(from, to)}?`, | |||
| choices: _.compact([ | |||
| {name: 'Yes', value: true}, | |||
| {name: 'Update immediately and create a separate commit', value: 'commit'}, | |||
There was a problem hiding this comment.
Let's better move it below No option
|
|
||
| // Showing the list of modules that are going to be updated | ||
| console.log( | ||
| `\n${strong('These packages will be updated:')}\n\n` + |
There was a problem hiding this comment.
It will always show a single package, right?
| '\n' | ||
| ); | ||
|
|
||
| const {indent} = detectIndent(packageSource); |
There was a problem hiding this comment.
Let's move this block of code into a separate function e.g. updatePackageJson
| ); | ||
|
|
||
| try { | ||
| if (existsSync('yarn.lock')) { |
There was a problem hiding this comment.
We don't need to make this check every time, right? It can be done only once.
| execSync('git add package.json package-lock.json', {stdio: 'inherit'}); | ||
| } else { | ||
| // Default to only using npm install and not including a lock file | ||
| execSync('npm install', {stdio: 'inherit'}); |
There was a problem hiding this comment.
I don't think it's right to generate a package-lock.json if it wasn't there before. There is an npm flag to not generate it during npm install. Shall we use it?
| } | ||
| execSync(`git commit -m "Upgrade ${name} from ${from} to ${to}"`,{stdio: 'inherit'}); | ||
| // Clean the list of packages to be updated after updating and commiting | ||
| updatedModules.splice(0, updatedModules.length); |
| // Clean the list of packages to be updated after updating and commiting | ||
| updatedModules.splice(0, updatedModules.length); | ||
| } catch(err) { | ||
| console.error(err) |
There was a problem hiding this comment.
I think we need to show more user-friendly errors e.g. console.error(`Error upgrading ${name} module: ${err.message}`)
|
Good feedback, I'll see when I have time to continue working on this. |
Based on my suggestion in #37, here is a PR that implements that functionality.
This is still a bit of work in progress, but I wanted to open this PR in order to get feedback. I used this solution now for my project and it seems to be working well for my use case.
Here's the list of things to do:
Implements and closes #37