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

Renames notification-center methods #120

Merged
merged 3 commits into from
Jul 2, 2018

Conversation

rashidsp
Copy link
Contributor

Renames 'clearNotifications' to 'clearNotificationListeners' and 'cleanAllNotifications' to 'clearAllNotificationListeners'.
Added deprecated methods along '@deprecated' meta tag.

@coveralls
Copy link

coveralls commented Jun 29, 2018

Coverage Status

Coverage decreased (-0.04%) to 96.996% when pulling 62223ae on rashid/NC-deprecated-methods into 29db920 on master.

@@ -407,10 +407,35 @@ function () {
);
}

public function testClearNotifications()
public function testclearNotificationsAndVerifyThatclearNotificationListenersWithArgsIsCalled()
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: should be testClear...

$this->notificationCenterMock->clearNotifications(NotificationType::ACTIVATE);
}

public function testclearNotificationListeners()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same typo here

$notificationCenterA->clearAllNotificationListeners();
}

public function testcleanAllNotificationsAndVerifyThatclearAllNotificationListenersIsCalled()
Copy link
Contributor

Choose a reason for hiding this comment

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

testClean...

$this->notificationCenterMock->expects($this->once())
->method('clearAllNotificationListeners');

$this->notificationCenterMock->cleanAllNotifications();
}

public function testsendNotificationsGivenLessThanExpectedNumberOfArguments()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably fix this as well

@mikeproeng37
Copy link
Contributor

build

@mikeproeng37
Copy link
Contributor

build

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

lgtm

@mikeproeng37
Copy link
Contributor

Known feature test failure

@mikeproeng37 mikeproeng37 merged commit 9984ac5 into master Jul 2, 2018
@oakbani oakbani deleted the rashid/NC-deprecated-methods branch July 3, 2018 04:43
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