Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the callback option #158

Merged
merged 6 commits into from
Dec 12, 2019
Merged

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
Member

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
Member

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
Member

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
Member

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.

readme.md Outdated Show resolved Hide resolved
@LitoMore
Copy link
Contributor Author

LitoMore commented Jul 1, 2019

@sindresorhus Updated

readme.md Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

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