feat: add a CLI message when newer version is available #292
Conversation
src/cli/index.ts
Outdated
| // Checks for available update and returns an instance | ||
| // Default updateCheckInterval is once a day | ||
| var notifier = updateNotifier({ | ||
| pkg: pkg, |
There was a problem hiding this comment.
You can write this as `updateNotifier({pkg}); when object key is same as a variable it points to, you can use this syntactic sugar :-)
src/cli/index.ts
Outdated
| var notifier = updateNotifier({ | ||
| pkg: pkg, | ||
| }); | ||
| // Notify using the built-in convenience method |
There was a problem hiding this comment.
I'm not sure I understand this comment. Do you think we need it?
src/cli/index.ts
Outdated
| import * as updateNotifier from 'update-notifier'; | ||
| var pkg = require('../../package.json'); | ||
|
|
||
| async function updateCheck() { |
There was a problem hiding this comment.
As notify is not asycnhronous (i.e. we don't do await notifier.notify(), async keyword before the function name is misleading.
src/cli/index.ts
Outdated
| @@ -108,6 +121,7 @@ function checkPaths(args) { | |||
| } | |||
|
|
|||
| async function main() { | |||
| await updateCheck(); | |||
There was a problem hiding this comment.
Related to the comment above, updateCheck() can be fully synchronous.
src/cli/index.ts
Outdated
| @@ -14,6 +14,19 @@ import errors = require('../lib/error'); | |||
| import ansiEscapes = require('ansi-escapes'); | |||
| import {isPathToPackageFile} from '../lib/detect'; | |||
|
|
|||
| import * as updateNotifier from 'update-notifier'; | |||
| var pkg = require('../../package.json'); | |||
src/cli/index.ts
Outdated
| @@ -14,6 +14,19 @@ import errors = require('../lib/error'); | |||
| import ansiEscapes = require('ansi-escapes'); | |||
| import {isPathToPackageFile} from '../lib/detect'; | |||
|
|
|||
| import * as updateNotifier from 'update-notifier'; | |||
| var pkg = require('../../package.json'); | |||
There was a problem hiding this comment.
Personal sidenote - I don't like using require to load .json files, I would suggest fs.readFile, but I fear we can saw this pattern in this app more often, so it's probably good to go.
yuliabaron
force-pushed
the
feat/cli_upgrade_message
branch
from
97112f1
to
061388b
Compare
yuliabaron
force-pushed
the
feat/cli_upgrade_message
branch
from
061388b
to
1c2bd4a
Compare
src/cli/index.ts
Outdated
| try { | ||
| notifier = updateNotifier({pkgFile}); | ||
| } catch (err) { | ||
| console.warn('Failed to start updater, ERR=', err); |
There was a problem hiding this comment.
Not sure whether to keep this warning. The code works from different directories, errors were observed during the tests on travis only
There was a problem hiding this comment.
I imagine that we get here when version is not present in the package.json as would be the case on Travis etc.
| return; | ||
| } | ||
|
|
||
| notifier.notify(); |
There was a problem hiding this comment.
notifier could be null here due to let and try/catch.. perhaps move notifier.notify(); up into the try block.
There was a problem hiding this comment.
i return from catch, but you're right, will move
yuliabaron
force-pushed
the
feat/cli_upgrade_message
branch
from
1c2bd4a
to
5053c4e
Compare
|
🎉 This PR is included in version 1.116.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Adding a message to CLI when a newer version is available
What are the relevant tickets?
https://snyksec.atlassian.net/browse/SC-6399
Screenshots