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

Send Message to User #8

Merged
merged 1 commit into from Aug 27, 2016
Merged

Send Message to User #8

merged 1 commit into from Aug 27, 2016

Conversation

ahmedash95
Copy link
Contributor

@ahmedash95 ahmedash95 commented Aug 22, 2016 •
edited

based on #2 , in this PR you can send a notification to user or twitter timeline

return new TwitterMessage('ahmedash95','Test twitter laravel notification');

of course if you need to post it on timeline just use one parameter

return new TwitterMessage('Test twitter laravel notification');

@christophrumpel
Copy link
Collaborator

Great and thx. I'm right now at the Laracon but I will try to check it out the next days.

@ahmedash95
Copy link
Contributor Author

ahmedash95 commented Aug 22, 2016 •
edited

oh , have a nice time there , waiting your next response :)

@christophrumpel
Copy link
Collaborator

So finally I found some time =) First thx for the PR. I encountered an error due to the count() functions. You did the check like this if(count($args == 2)) but it should be like `if(count($args) == 2)' . I changed that for me and then it worked. This is also why all the tests failed. Could you change that and update the PR?

Besides that I really like it =)

@christophrumpel
Copy link
Collaborator

And please take a look at the the styleci errors https://styleci.io/analyses/XaV0YY and change your code to that style.

@ahmedash95
Copy link
Contributor Author

@christophrumpel i got shocked :D , don't know how php if(count($args == 2)) worked with me before .. now everything is okay as i guess.
PS . thanks for your nice words :)

@christophrumpel christophrumpel merged commit 466203f into laravel-notification-channels:master Aug 27, 2016
@christophrumpel
Copy link
Collaborator

Thx @ahmedash95 . Everything is working now 👍

@ahmedash95
Copy link
Contributor Author

ahmedash95 commented Aug 27, 2016 •
edited

😄 Thx @christophrumpel

@ahmedash95 ahmedash95 deleted the message-to-user branch August 27, 2016 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ahmedash95 @christophrumpel