search

LaraBug / LaraBug

Laravel error reporting tool
https://www.larabug.com
MIT License
267 stars 58 forks source link

Auto registering log channel #58

Closed HajMo closed 2 years ago

HajMo commented 3 years ago

This pull request will enable LaraBug without registering it manually as it's described in installation guide in project details page.

Cannonb4ll commented 3 years ago

What if users want to disable it?

HajMo commented 3 years ago

They can delete the login_key value in .env or we can add a new env variable to decide if the service is enabled.

SebastiaanKloos commented 3 years ago

@HajMo There should be an easy option to disable this feature as the October CMS plugin would break when merging this PR. Configuring config files is very personal and that's possibly why this does not exist in the package. @Cannonb4ll what do you think?

Cannonb4ll commented 3 years ago

Yes I agree, I'm not too sure about this PR, of course, it is nice, but I recon it'll do more harm rather than it being helpful.

HajMo commented 3 years ago

@SebastiaanKloos We can add more checks in the condition with config option that decide if this feature is enabled, something like:

if (config('larabug.login_key') && config('larabug.auto_register')) {
    // Register
}

And in config/larabug.php there's will be boolean auto_register variable. I think this will be a solution.

Cannonb4ll commented 3 years ago

I'm still quite unsure whether to merge this at this point.

HajMo commented 3 years ago

@SebastiaanKloos What you think?

SebastiaanKloos commented 2 years ago

@SebastiaanKloos What you think?

As @Cannonb4ll already said, we are still unsure whether we are going to merge this.

Let's leave this PR open for now and see if more users want this merged.

lloricode commented 2 years ago

Yes I agree, I'm not too sure about this PR, of course, it is nice, but I recon it'll do more harm rather than it being helpful.

You have point, let developer manage this, so developer will aware of configuration rather than register automagically

SebastiaanKloos commented 2 years ago

@HajMo I appreciate the work you've put in the PR. However, we are not going to implement this in the near future.

I recommend forking the repo and adding your functionality, or creating your own package requiring the larabug/larabug package. Which automatically sets the logging configuration.

@Cannonb4ll @Glennmen do you agree?

Cannonb4ll commented 2 years ago

Yes, for now I agree. Once this gets more replies, comments and genuine use-case examples I'd be glad to take another look at it.