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

Fix for Laravel 5.5 #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

RohanSakhale
Copy link

No description provided.

@marcinj1
Copy link

Please update. I'm using your package and i'm getting error


$this->app['maknz.slack'] = $this->app->share(function ($app) {
$this->app['maknz.slack'] = $this->app->singleton('maknz.slack', function ($app) {

Choose a reason for hiding this comment

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

I don't think this will work . You don't need to reassign $this->app['maknz.slack'] with the returned value from app->singleton since you already define the name for the singleton as a parameter to the function

Copy link
Author

Choose a reason for hiding this comment

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

Could you please review my change and accept, have tested locally over tinker, Slack command works

@RohanSakhale
Copy link
Author

@skmetaly also listed Laravel 5.5, auto package discovery in composer.json

@skecskes
Copy link

skecskes commented Dec 27, 2017

I just made a fork and fixed it there: if anybody is interested: https://github.com/phpify/slack-laravel or just composer require phpify/slack-laravel

@jeremykenedy
Copy link

The package is no longer being maintained... here is my fork of it..

composer require jeremykenedy/slack-laravel

https://github.com/jeremykenedy/slack-laravel

@qmcree
Copy link

qmcree commented Jan 17, 2018

This PR worked well for me and fixed the Laravel 5.5 incompatibility issues. It should be accepted. :) Thanks, @RohanSakhale!

@cedricve
Copy link

Please merge.

@RohanSakhale
Copy link
Author

@skmetaly could you please merge this PR

@dannytrue
Copy link

just implemented the same fix on my local and then found the same solution in PR.

@maurocasas
Copy link

@maknz WTF ARE YOU WAITING FOR TO MERGE THIS.

@qmcree
Copy link

qmcree commented May 19, 2018

@maurocasas You don’t need to wait on him. Just update the dependency in your package manager to be constrained to this specific commit hash.

@maknz
Copy link
Owner

maknz commented Jan 21, 2019

If you took 30 seconds to glance at the README, you'd see this package is no longer being maintained as of early 2017. Use one of the (100+) forks that implements the features you want, or make your own.

Screaming at me to "WTF ARE YOU DOING MERGE THIS" only serves to discourage me from ever contributing open source code again.

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.

10 participants