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

[WIP] Moves validator middleware into another package #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Moves validator middleware into another package #74

wants to merge 1 commit into from

Conversation

neeckeloo
Copy link

@neeckeloo neeckeloo commented Oct 18, 2017

I suggest to move validator middleware into another package in order to use it in another context without the need to import the bundle. I created a package with the middleware that can be integrated in the League organization.
The validator middleware package has been defined as suggested package in the composer file as is currently not used by default.

BC breaks:

  • rename middleware from League\Tactician\Bundle\Middleware\ValidatorMiddleware to League\Tactician\Validator\ValidatorMiddleware

@neeckeloo
Copy link
Author

neeckeloo commented Oct 19, 2017

@rosstuck What do you think about? Build is currently failing due to issue related to composer repository.

@rosstuck
Copy link
Member

Hi @neeckeloo, thanks for opening this PR!

Can you tell me a bit more about your usecase? To be completely honest, after having split a couple of these things out, it kind of doesn't feel like the hassle. Every repo is a new set of license files, another travis config, another conditional compiler pass + tests, another set of version dependencies and headaches, etc.

For folks who'd like to use the validator middleware standalone (which is really trivial to recreate), they can install the bundle package and just use the class they want from it. In Tactician 2.0, there's a pretty good chance the PSR Logger and Container plugins will just be in the package directly because they're really thin.

I'm not trying to knock your work here (which I do appreciate it!), so I apologize for sounding super negative. I'm just want to know more about your usecase before I make any decision and to share upfront any resistance I have so you get where I'm coming from. :)

Thanks in advance!

@neeckeloo
Copy link
Author

@rosstuck Thank you for your answer.

I fully understand your point of view and I knew that everything was kept in the same package to simplify maintenance.

In my case, I would need the middleware in a Zend Framework-based project and I have to retrieve all Symfony dependencies defined in the bundle:

  • symfony/config
  • symfony/dependency-injection
  • symfony/http-kernel
  • symfony/yaml

I regret the fact of having to redefine this middleware in my code even if it remains very trivial and I like the principles explained by Matthias Noback about packages that consist in part to decouple things.

Give the number of frameworks in the PHP ecosystem nowadays, it would be a shame to copy / paste or rewrite the same thing over and over for vendor specific wrapper, hence the proposal of having a separate repository. Moreover, I provide you a ready-to-use package that will probably not require much maintenance in the future.

@rosstuck
Copy link
Member

Hi @neeckeloo, thanks for the feedback! Thanks for letting me know more about your usecase.

I don't have time this week to take any further action with this right now and I'd like to leave the issue open a bit more to how common this use case is. 😄

Also, FYI, if we do split this out, I'll probably just start with a new package from scratch rather than integrating/moving the repo you've created. The code in the package you've provided is copy and pasted from this one and since I need to reregister all the different services it's probably easier to just start with a fresh League repo.

If you have any other feedback, please let me know!

@ogizanagi
Copy link
Contributor

ogizanagi commented Nov 5, 2017

Actually the Symfony bundle and the middlewares could have been part of the thephpleague/tactician repo under a Bridge namespace IMHO. That's more and more common, promoting component-first approach and providing relevant integrations in the same repository with soft dependencies and it's easier to maintain.

See:

@ogizanagi ogizanagi mentioned this pull request Nov 13, 2017
17 tasks
@lcobucci
Copy link

Actually the Symfony bundle and the middlewares could have been part of the thephpleague/tactician repo under a Bridge namespace IMHO.

Having that wouldn't solve the installation of useless dependencies. I'm using tactician on applications that need to be very thin and having symfony/framework-bundle and other packages is not really nice...

@ogizanagi
Copy link
Contributor

ogizanagi commented Nov 13, 2017

symfony/framework-bundle, symfony/validator and other dependencies would be soft-dependencies (require-dev only for thephpleague/tactician package for tests, and in suggestions to use a feature or framework integration).

@lcobucci
Copy link

@ogizanagi that works fine for me, what can we do to help on this?

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.

4 participants