-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added notification methods #115
Conversation
- Added switch links enable/disable notifications - Added interface links enable/disable notifications
-Fixed lint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Alopalao good job how you've introduced the methods, they have very well defined responsibilities and easy to test. Overall looks good, but we'll need also to cover some points that I mentioned, thanks.
Aldo, when you have a chance, please update the changelog summarizing the notification changes you're introducing. Let me know if you've also explored it locally, looks like it's almost ready to land. Thanks. |
Yeah, here are the logs. I changed the log from
|
- Updated readme.rst
When I merged #114, it auto deleted the branch that this PR depends on, I just restored it, and updated this PR to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic contribution, @Alopalao
Closes #71