zendframework / zend-expressive-authentication

Authentication middleware for Expressive and PSR-7 applications
BSD 3-Clause "New" or "Revised" License
21 stars 15 forks source link

The \Zend\Expressive\Authentication\UserRepositoryPdoDatabaseFactory factory should accept PDO instance #41

Closed nuxwin closed 5 years ago

nuxwin commented 5 years ago

@weierophinney

Good morning,

Right now, the \Zend\Expressive\Authentication\UserRepositoryPdoDatabaseFactory factory force us to provide whole configuration set for PDO. Most of time, we will have our own factory returning a PDO instance already. It could be great if the \Zend\Expressive\Authentication\UserRepositoryPdoDatabaseFactory factory accept a service name for the PDO, or even a PDO instance.

Thank you.

froschdesign commented 5 years ago

@nuxwin It's not needed to pinging @weierophinney or someone else explicitly. Thanks!

nuxwin commented 5 years ago

@froschdesign

Yes... No problem. You can continue to sleep... It's because of guys like you that the Zend Framework project will die ;) And I'm really serious with that sentence ;)

froschdesign commented 5 years ago

@nuxwin And you think the comment would be helpful now?! Funny, because you do not know what I'm doing or not.

offtopic ends here

nuxwin commented 5 years ago

@froschdesign

Sorry but what I see for years:

  1. The issues are not addressed. The issues are pushed in standbye, mostly ignored by the project members...
  2. The members of the project deliver components that they don't maintain ... They prefers to deliver new components that they will not maintain more... (MVC, now expressive...)
  3. When we ping them to try to go ahead, we get comment like the one you have done...

You should try to understand why Zend Framework is at bottom now compared to other projects like Laravel and Symfony... Many people like me don't contribute much to the project (with PR) because we know all that our works will stay in standbye for months/years and will be deliberately ignored by the project members.

That explains why I'm a little angry.

geerteltink commented 5 years ago

... and will be deliberately ignored by the project members.

I think I can speak for most maintainers when I say that we are not deliberately ignoring PR's or issues. Most of us are devoting our free time to maintain this project. And that free time is exactly what you never have enough so you need to make priorities. I don't know about you or other contributors and maintainers, but I rather keep up with my family and social life then sitting behind a screen all night and replying to issues like this.

The reason some PR's are open for a longer time is that we don't have time or it might lack tests. Merging a PR takes time, even a small change takes time because we need to check the code, understand and think about the changes and possible BC breaks, test the code, merge it to the master / develop branch, create a release, prepare the next release... So it's faster to let some new feature PR's build up for a package and merge and release everything at once. Important bugfixes and security issues are always merged and released within hours when possible.

/ontopic Instead of requesting a new feature, why not supply a PR with tests in place so we can check, merge and release it.

nuxwin commented 5 years ago

/ontopic Instead of requesting a new feature, why not supply a PR with tests in place so we can check, merge and release it.

Because I know the PR's won't be reviewed nor merged... You can say whatever you want. The facts are the facts ;) Sure, my sentence can looks a bit offensive but:

  1. Zend MVC: An opinionated big blackbox which most of developers will throw away because it is too difficult to understand, too long to learn, too much event-driven, and even worse, no longer recommended by their own creators: Welcome expressive.
  2. Zend Expressive: A great micro-framework but which seem far too young (too fiew components and so on...)
weierophinney commented 5 years ago

@nuxwin —

The reason @froschdesign said there is no reason to ping somebody is because multiple maintainers exist for every component. Pinging people when creating an issue is not necessary, and should only be done when the person mentioned requested it (e.g. "Per a request from @soandso in slack" or "To resolve an issue reported by @soandso").

Second, we actually prioritize pull requests over issues. Why? Because, at the very least, they should contain a test case that reproduces the issue, even if no solution is presented. Having a reproducible test case allows us to better understand the problem, and whether or not it presents an actual issue, or a problem in our documentation. For features, it demonstrates how the author expects to consume the new feature. In both cases, it allows us to have a dialog over approach, and guide the contributor towards a solution we find mutually maintainable.

Third, ranting at the maintainers of the framework is never productive, and likely will make those maintainers less likely to consider your issues or pull requests.

Finally, and related, this sort of discussion should happen in the Slack or forums, NOT as part of commentary on an issue.


To address the issue actually reported: this would have been a good question to ask in the Slack, so that you knew which approach to use for a pull request.

For BC purposes, I'd argue allowing a configuration key that specifies the service returning a PDO instance would make most sense. We could check for this first in the factory, and, if set and it exists, pull it and return it immediately.

nuxwin commented 5 years ago

@weierophinney

You're right in all points but I'm really confused since years now about ZF... ZF provide a great value for our projects but in the meantime, its development seem really less active than the other frameworks (Laravel, Symfony). I think that your development policy is one of the main reason. If someone is making a pull request and see it ignored for far too long, he will simply go away and will not contribute further... I've always preferred ZF because I think it follows better coding standards, even if often, available components are far too generic and don't provide implementation for real use cases (For instance: The ACL and the RBAC components versus the Symfony security bundle and all related bundles). Laravel, I prefer don't talk about it.... It look too much like a toy to me. Symfony... I really dislike their DI implementation.... and how all the things get configured...

So ok, I'll make a PR here but I really hope that this will not be ignored for months/years...

Thank you and sorry again.