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.

Sign up for GitHub

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

Jump to bottom

Add notifications new signature #6671

Merged
merged 7 commits into from
Oct 14, 2021
Merged

Add notifications new signature #6671

merged 7 commits into from
Oct 14, 2021

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented Oct 13, 2021

Follow up of #6191

@WiXSL WiXSL mentioned this pull request Oct 13, 2021
Merged
djhi
djhi requested changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think we should probably also display a deprecation warning when the second argument isn't an option object

@WiXSL WiXSL requested a review from djhi October 13, 2021 18:58
djhi
djhi approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@djhi
Copy link
Contributor

djhi commented Oct 14, 2021

It needs rebase though

@djhi
Copy link
Contributor

djhi commented Oct 14, 2021

It seems there is an issue with your commits

@WiXSL
Copy link
Contributor Author

WiXSL commented Oct 14, 2021

I kind of messed up the merge, but all the changes are there

@djhi
Copy link
Contributor

djhi commented Oct 14, 2021

Can you clean it up?

@WiXSL WiXSL force-pushed the add-notification-new-signature branch from 415b742 to 37b9154 Compare October 14, 2021 14:07
@WiXSL WiXSL requested a review from djhi October 14, 2021 14:19
@djhi djhi added this to the 3.19 milestone Oct 14, 2021
@djhi djhi merged commit 315e82e into marmelab:master Oct 14, 2021
@djhi
Copy link
Contributor

djhi commented Oct 14, 2021

Thanks!

@WiXSL WiXSL deleted the add-notification-new-signature branch October 14, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djhi djhi Awaiting requested review from djhi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.19
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@WiXSL @djhi