Bail if notify is disabled

[wejendorp]

Avoids initializing configStore and showing related errors if environment variable or argv disables notifier.

Rel #107

[nexdrew]

This looks good to me, but it'd be nice to include tests for the NO_UPDATE_NOTIFIER env var and --no-update-notifier argv switch.

[wejendorp]

Yea I'll add tests for it. 👍

[wejendorp]

Not sure how to group the specs with the setup / restore. What do you think?

[nexdrew]

Thanks for adding the tests! I found a slight scoping error (please see requested changes below), but otherwise this looks great to me.

Note that this repo doesn't officially have any code coverage built-in for testing, but nyc reports a slight improvement with this PR, from 84.38% to 84.62%.

[nexdrew]

I think we actually need to move this logic within the beforeEach closure; otherwise line 32 only preserves the original array after the first test, meaning that line 57 is modifying the shallow copy and the original array is lost, which affects subsequent tests (even though it doesn't cause the tests to fail, it does affect code coverage).

I suggest removing line 22 and making the subsequent lines look like this:

	let argv;
	let configstorePath;

	beforeEach(() => {
		argv = process.argv.slice();
		configstorePath = updateNotifier(generateSettings()).config.path;
	});

[wejendorp]

I figured the original argv could be captured just once before running the specs. But I am honestly not 100% that that's what happens.
Will update.

[nexdrew]

Yeah I thought the same thing at first (and was really confused when code coverage dropped after adding your tests), but unfortunately doing process.argv = argv means we have to get a new shallow copy each time to avoid side effects of process.argv.push(/*...*/) in subsequent tests (since both variables reference the same array object).

[wejendorp]

Oh yea, damn that makes sense. Nice catch.

[nexdrew]

LGTM 👍 Thanks for the contribution!

I'd like to get approval from one of the owners before merging though, since I'm not super active in this repo and they'll need to publish a new release to npm anyway.

[nexdrew]

@SBoudrias @sindresorhus See any problems with this change? Instead of only short-circuiting check(), this expands the NO_UPDATE_NOTIFIER/--no-update-notifier scope to also short-circuit config initialization in the constructor.

[SBoudrias]

Looks good for me too. Thanks for handling the review @nexdrew!!

Thanks for the contribution @wejendorp!

[nexdrew]

@SBoudrias No problem, thanks for merging and releasing this!

@wejendorp Thanks again for the contribution!