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

Proposal: Register upload handlers in config #50

Closed
nerg4l opened this issue Sep 6, 2018 · 4 comments
Closed

Proposal: Register upload handlers in config #50

nerg4l opened this issue Sep 6, 2018 · 4 comments

Comments

@nerg4l
Copy link

nerg4l commented Sep 6, 2018

It would be great if the handler registration would be in the config file intead in the HandlerFactory. So handlers could work similarly to providers. This would make the library more extendable.

  • Ability to remove unused handlers
  • Developers could extend the library easyer (without the need of implementing the whole library)

What do you think about this?

@pionl
Copy link
Owner

pionl commented Sep 7, 2018

Hi,

you already do not need to implement the whole library, you can call Handler:: register(MyOwnImplementation::class), the docs are in Wiki

Anyway, you have a point. It would be easier to provider entry in config. I would not recommend to have in-built handlers in config (updating would be harder), but we could allow user to override the handlers. This should be pretty easy.

... config
'handlers' => [
// A list of handlers/providers that will be appended to existing list of handlers
'custom' => []
// Overrides the list of handlers - use only what you really want
'override' => []
]

@nerg4l
Copy link
Author

nerg4l commented Sep 7, 2018

Hello,

You are right. It seems I did not read carefully enough and missed that part. Currently I registered the library as a local repository with composer to be able to add a handler.

You are right about the problems with update process. And this configuration idea looks promissing.

@pionl
Copy link
Owner

pionl commented Sep 10, 2018

Working on it

@pionl pionl closed this as completed in 4507809 Sep 11, 2018
@pionl
Copy link
Owner

pionl commented Sep 11, 2018

I've updated the wiki:

Integration is updated to use config. Service provider is now partly unit tested (to cover this change).

pionl added a commit that referenced this issue Sep 11, 2018
pionl added a commit that referenced this issue Sep 11, 2018
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

No branches or pull requests

2 participants