thephpleague / statsd

A library for working with StatsD
MIT License
351 stars 56 forks source link

How to approach Laravel 5 pull request? #20

Closed casperbakker closed 8 years ago

casperbakker commented 9 years ago

The Laravel Service Provider of this package does not work with Laravel 5. I could easily make a Pull Request to add support for Laravel 5.

Would you all prefer a new League\StatsD\Laravel5\Facade\StatsdFacade or just upgrade the current Laravel facade and break compatibility with Laravel 4?

I think a new Facade for Laravel5 would be nicer, to avoid breaking old installs.

casperbakker commented 9 years ago

I mean ServiceProvider... :)

tomschlick commented 9 years ago

Any progress on a L5 service provider?

isbkch commented 8 years ago

Any updates on this ? It's still not working on 5.1

casperbakker commented 8 years ago

Well, because I did not get a response, I just made my own Service provider in our app. This is the code:

<?php namespace App\Providers;

use Illuminate\Support\ServiceProvider;
use League\StatsD\Client as Statsd;

class StatsdServiceProvider extends ServiceProvider {
    public function boot()
    {
        //
    }

    public function register()
    {
        $this->registerStatsD();
    }

    protected function registerStatsD()
    {
        $this->app['statsd'] = $this->app->share(
            function ($app) {
                // Set Default host and port
                $options = array();
                if (isset($app['config']['statsd.host'])) {
                    $options['host'] = $app['config']['statsd.host'];
                }
                if (isset($app['config']['statsd.port'])) {
                    $options['port'] = $app['config']['statsd.port'];
                }
                if (isset($app['config']['statsd.namespace'])) {
                    $options['namespace'] = $app['config']['statsd.namespace'];
                }
                // Create
                $statsd = new Statsd();
                $statsd->configure($options);
                return $statsd;
            }
        );
    }
}
marcqualie commented 8 years ago

Apologies for the delay. I've never used Laravel before so I'm not in a position to review this. There is a pull request open (#21) so if someone wants to verify that it works and is a valid provider then I can merge that.

isbkch commented 8 years ago

I've ended up implementing manually the changes in the pull request and it works just fine. One thing I had to change though: