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

Make cmixin/business-day configurable via Laravel config file #29

Closed
kylekatarnls opened this issue Mar 31, 2019 · 1 comment
Closed
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@kylekatarnls
Copy link
Owner

kylekatarnls commented Mar 31, 2019

Laravel auto-discovery could be used to automatically run BusinessDay::enable('Illuminate\Support\Carbon'); then a file in config directory could be used to pass options. So Laravel users would not have to run global code on bootstrap.

A great solution would be to create a feature in Carbon first to read composer.json of other packages in the vendor directory, then retrieve the extra.carbon config from it that would indicates to Carbon the packages contains macros/mixins and how to enable and configure them. Then Carbon itself would enable them automatically on the Laravel boot it already has and could even call them in some custom boot method with any other framework.

A fallback solution would be to create class Cmixin\BusinessDay\BusinessDayServiceProvider extending Illuminate\Support\ServiceProvider with the enable() call inside the boot() method and a $this->mergeConfigFrom() inside the register().

Then adding the auto-discovery in composer.json

  "extra": {
    "laravel": {
      "providers": [
        "Cmixin\\BusinessDay\\BusinessDayServiceProvider"
      ]
    }
  }

Good example here: https://github.com/aaroncadrian/carbon-macros/blob/master/src/CarbonServiceProvider.php

Note: Same feature would be added on https://github.com/kylekatarnls/business-time and we should check both can be configured with no conflict.

See kylekatarnls/business-time#10

@kylekatarnls
Copy link
Owner Author

The realization of this is not planned yet. So if someone is interested in making a pull-request for this, it would be a pleasure to help her/him to contribute for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant