Skip to content

Conversation

@LitoMore
Copy link
Contributor

@LitoMore LitoMore commented May 16, 2019

Just deprecate the callback option as #85 (comment) mentioned.

It's a breaking change.

@LitoMore LitoMore changed the title Deprecated callback option Deprecate callback option May 16, 2019
@sindresorhus
Copy link
Owner

You are not deprecating it, you are removing it.

@sindresorhus sindresorhus changed the title Deprecate callback option Remove the callback option May 18, 2019
@sindresorhus
Copy link
Owner

And you missed this point:

and add a updateNotifier.check() method that returns a Promise for the update.

@LitoMore
Copy link
Contributor Author

LitoMore commented May 18, 2019

@sindresorhus The previous code only does checkNpm() when the callback option is present, so I have not idea how to refactor this part.

and add a updateNotifier.check() method that returns a Promise for the update.

We already have the updateNotifier.checkNpm(), we could use the updateNotifier.checkNpm() directly to get the version information.

@sindresorhus
Copy link
Owner

We already have the updateNotifier.checkNpm(), we could use the updateNotifier.checkNpm() directly to get the version information.

Yes, but I think it needs a better name. Maybe checkImmediately() or something better. And it needs to be documented.

@LitoMore
Copy link
Contributor Author

Yes, but I think it needs a better name. Maybe checkImmediately() or something better. And it needs to be documented.

This method is doing something about get some information. Maybe we could name this method getSemverDiff()?

@sindresorhus
Copy link
Owner

I don't like the name getSemverDiff(). It's not clear what it does or its purpose, it's also not accurate as the return value is not just the diff.

@LitoMore
Copy link
Contributor Author

LitoMore commented Jul 1, 2019

@sindresorhus Updated

@sindresorhus
Copy link
Owner

Can you also fix the merge conflict?

@LitoMore
Copy link
Contributor Author

Can you also fix the merge conflict?

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants