Skip to content

Conversation

dennisdegreef
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong. If it is in the Activity subnamespace, it should be acess from an Api\Activity API, not from the main client, to be consistent with the current architecture of the library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to namespace Api\Notification see 300a091

Copy link
Contributor

Choose a reason for hiding this comment

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

typehinting the interface means that it is not compatible with PHP 5.3 or 5.4

@stof
Copy link
Contributor

stof commented Feb 10, 2015

Tests are missing

@dennisdegreef
Copy link
Contributor Author

Added test and fixed PHP5.3 and 5.4 compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

The a should be removed here.

pilot added a commit that referenced this pull request Feb 18, 2015
Implemented notifications from github API
@pilot pilot merged commit fd6a5f9 into KnpLabs:master Feb 18, 2015
@pilot
Copy link
Contributor

pilot commented Feb 18, 2015

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.

4 participants