New: Notify users if the current version of sonar is not up to date #423

qzhou1607-zz · 2017-08-14T21:55:56Z

Currently the message looks as below:

alrra · 2017-08-14T22:45:09Z

src/lib/cli.ts

+    const changelogUrl: string = `https://sonarwhal.com/about/changelog.html`;
+    // No indentation due to the use of `\` to avoid new line.
+    // https://stackoverflow.com/a/35428171
+    const message: string = `Update available ${chalk.dim(update.current)}${chalk.reset(' → ')}${chalk.green(update.latest)}\nRun ${chalk.cyan(`npm i -g ${update.name}`)} to update\nSee ${chalk.red(changelogUrl)} for update details`;


sonar may be installed as a (dev)dependence and run via a npm script, so suggesting npm i -g is not accurate in those cases.

@alrra If we have a label in the code that indicates whether or not sonar is installed as a dependency, I can generate different messages based on different scenarios. Does such "label" exist? Otherwise I can include both npm i -g and npm i -D options in the message and ask the user to pick according to their own use.

Otherwise I can include both npm i -g and npm i -D o

-P | --save-prod is also possible.

Otherwise I can include both npm i -g and npm i -D options in the message and ask the user to pick according to their own use.

Not all users will know the differences, so if we do that we will need to also explain the differences between the options, and IMHO that is not the way to go.

My opinion is that the less we asked from the user the better.

@alrra How about using detect-installed to see if sonar was installed globally first. If yes, then add the -g flag in there?

I would just say "please update" or similar. Don't think it is worth pulling another dependency just for that.

alrra · 2017-08-14T22:52:57Z

package-lock.json

@@ -380,28 +380,24 @@
      "version": "2.0.0",


These package-lock.json changes fluctuate so much. :(

@alrra Is there a good way to avoid fluctuation?

@qzhou1607 I was just making a remark, nothing to do with this PR.

These package-lock.json changes fluctuate so much. :(

From http://blog.npmjs.org/post/167963735925/v560-2017-11-27

bc263c3fd #19054 Fully cross-platform package-lock.json. Installing a failing optional dependency on one platform no longer removes it from the dependency tree, meaning that package-lock.json should now be generated consistently across platforms! 🎉

Update your npm install now 🎉

qzhou1607-zz · 2017-08-15T16:34:52Z

@molant @alrra The message is changed as shown below:

sarvaje · 2017-08-16T16:50:59Z

tests/lib/cli.ts

+test.serial(`User shouldn't be notified if the current version is up to date`, async (t) => {
+    let undefinedUpdate;
+
+    t.context.notifier.update = undefinedUpdate;


I think you should do directly t.context.notifier.update = void 0 or t.context.notifier.update = null instead of use a variable.

qzhou1607-zz · 2017-08-16T18:45:34Z

@sarvaje Done.

molant · 2017-08-16T20:52:52Z

src/lib/cli.ts

@@ -61,6 +69,23 @@ const setUpUserFeedback = (sonarInstance: Sonar, spinner: IORA) => {
    });
 };

+type Update = { // eslint-disable-line no-unused-vars


It's a bit weird to have this type here. Isn't there a type package for update-notifier?
If not I think the type should be declared elsewhere. @sarvaje thoughts?

It looks like there is a package @types/update-notifier.
Anyway, I think that all types should go to the folder /types

Then we should use that package instead of doing this. @qzhou1607 can you please add it as a dev dependency?

@molant @sarvaje Done.

molant · 2017-08-17T00:02:22Z

package.json

@@ -12,6 +12,7 @@
  },
  "bin": "./dist/src/bin/sonar.js",
  "dependencies": {
+    "@types/update-notifier": "^1.0.1",


This should be with the other types under dev dependencies.

alrra · 2017-08-17T12:27:14Z

src/lib/cli.ts

+    // No indentation due to the use of `\` to avoid new line.
+    // https://stackoverflow.com/a/35428171
+    const message: string = `Update available ${chalk.red(update.current)}${chalk.reset(' → ')}${chalk.green(update.latest)}\
+\nPlease update to the newest version ${chalk.green(update.latest)}\nSee ${chalk.cyan(changelogUrl)} for details`;


Do we need both Update available... and Please update to the newest version...? To me they basically specify the same thing.

molant · 2017-08-18T13:18:28Z

@qzhou1607 or @alrra can you please take care of the conflicts?

Fix webhintio#419

qzhou1607-zz · 2017-08-18T15:41:33Z

@molant @alrra Done.

qzhou1607-zz requested review from alrra, austinkelleher, molant and patrick-steele-idem as code owners August 14, 2017 21:55

qzhou1607-zz force-pushed the notify_new_version branch from 4b5d93b to 4066949 Compare August 14, 2017 22:37

alrra reviewed Aug 14, 2017

View reviewed changes

qzhou1607-zz force-pushed the notify_new_version branch from 4066949 to 471d6b3 Compare August 14, 2017 22:48

alrra reviewed Aug 14, 2017

View reviewed changes

qzhou1607-zz force-pushed the notify_new_version branch from 471d6b3 to 2387c89 Compare August 15, 2017 16:30

qzhou1607-zz force-pushed the notify_new_version branch 3 times, most recently from 5b8221a to 7f34d7e Compare August 15, 2017 17:29

sarvaje reviewed Aug 16, 2017

View reviewed changes

qzhou1607-zz force-pushed the notify_new_version branch from 7f34d7e to f28740b Compare August 16, 2017 18:44

sarvaje approved these changes Aug 16, 2017

View reviewed changes

molant reviewed Aug 16, 2017

View reviewed changes

qzhou1607-zz force-pushed the notify_new_version branch from f28740b to 806a6cf Compare August 16, 2017 23:09

molant requested changes Aug 17, 2017

View reviewed changes

alrra reviewed Aug 17, 2017

View reviewed changes

qzhou1607-zz force-pushed the notify_new_version branch from 806a6cf to 5aae5c4 Compare August 17, 2017 19:23

molant approved these changes Aug 18, 2017

View reviewed changes

New: Notify users of new sonar versions

4820c29

Fix webhintio#419

qzhou1607-zz force-pushed the notify_new_version branch from 5aae5c4 to 4820c29 Compare August 18, 2017 15:41

alrra closed this in e9e4a95 Aug 18, 2017

snyk-bot mentioned this pull request Nov 9, 2020

[Snyk] Upgrade cloudinary from 1.9.1 to 1.23.0 magnologan/sonar#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New: Notify users if the current version of sonar is not up to date #423

New: Notify users if the current version of sonar is not up to date #423

qzhou1607-zz commented Aug 14, 2017 •

edited

Loading

alrra Aug 14, 2017 •

edited

Loading

qzhou1607-zz Aug 14, 2017

alrra Aug 14, 2017 •

edited

Loading

qzhou1607-zz Aug 14, 2017

molant Aug 15, 2017

alrra Aug 14, 2017

qzhou1607-zz Aug 14, 2017

alrra Aug 14, 2017

alrra Nov 29, 2017 •

edited

Loading

molant Nov 29, 2017

qzhou1607-zz commented Aug 15, 2017

sarvaje Aug 16, 2017 •

edited

Loading

qzhou1607-zz commented Aug 16, 2017

molant Aug 16, 2017

sarvaje Aug 16, 2017

molant Aug 16, 2017

qzhou1607-zz Aug 16, 2017

molant Aug 17, 2017

alrra Aug 17, 2017 •

edited

Loading

molant commented Aug 18, 2017

qzhou1607-zz commented Aug 18, 2017

New: Notify users if the current version of sonar is not up to date #423

New: Notify users if the current version of sonar is not up to date #423

Conversation

qzhou1607-zz commented Aug 14, 2017 • edited Loading

alrra Aug 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Aug 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Nov 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qzhou1607-zz commented Aug 15, 2017

sarvaje Aug 16, 2017 • edited Loading

Choose a reason for hiding this comment

qzhou1607-zz commented Aug 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

molant commented Aug 18, 2017

qzhou1607-zz commented Aug 18, 2017

qzhou1607-zz commented Aug 14, 2017 •

edited

Loading

alrra Aug 14, 2017 •

edited

Loading

alrra Aug 14, 2017 •

edited

Loading

alrra Nov 29, 2017 •

edited

Loading

sarvaje Aug 16, 2017 •

edited

Loading

alrra Aug 17, 2017 •

edited

Loading