doctrine / doctrine-laminas-hydrator

Doctrine hydrators for Laminas applications

https://www.doctrine-project.org/projects/doctrine-laminas-hydrator.html

MIT License

33 stars 19 forks source link

Updated docs #28

Closed driehle closed 2 years ago

driehle commented 2 years ago

As suggested by @greg0ire in #22, docs should always assume the latest PHP version. This PR updates the docs in several regards:

All Doctrine annotations have been rewritten from DocBlock comments to PHP 8 attributes
All classes have been rewritten to make use of typed properties (available from PHP 7.4)
Class strings have been replaced through ::class statements, classes have been imported
Examples for controllers have been updated to include EntityManager as a dependency in the constructor, because the call $this->getServiceLocator() inside controllers has already been deprecated for a very long time and has been dropped in laminas-mvc 3.0 (see release notes)

As said, this only affects docs. Code of the library has not been changed.

greg0ire commented 2 years ago

This PR is based on #22, so that one should be handled first.

22 should target 2.2.x, but this one should target 2.1.x, so IMO you should make it purely about docs.

driehle commented 2 years ago

@greg0ire

22 should target 2.2.x, but this one should target 2.1.x, so IMO you should make it purely about docs.

I don't think we should switch from annotations to attributes (in docs) in a bugfix release, hence, I'd rather put it on 2.2.x as well. In the end, we are promoting new best practices with this change.

The last two bullet points (class constants and ServiceLocator) could go on 2.1.x though...

greg0ire commented 2 years ago

I don't think we should switch from annotations to attributes (in docs) in a bugfix release, hence, I'd rather put it on 2.2.x as well. In the end, we are promoting new best practices with this change.

Why not? New developers are going to read documentation for 2.1.x for a while. IMO we should add commits to 2.2.x when there is a stability risk, but it's ok to commit to 2.1.x when changing files outside src. I think it's best if we want to avoid divergence between branches and conflicts.

driehle commented 2 years ago

Why not? New developers are going to read documentations for 2.1.x for a while. IMO we should add commits to 2.2.x when there is a stability risk, but it's ok to commit to 2.1.x when changing files outside src. I think it's best if we want to avoid divergence between branches and conflicts.

In general, yes, but doctrine-laminas-hydrator is not listed on Doctrine's website. Therefore, as soon as 2.2.0 is released and 2.2.x is the default branch, I don't think many people will read the old docs, as the docs are only visible directly on Github. That would - for sure - be different if they were listed on doctrine-project.org.

That said, this project should probably switch to the doc tools of the other Doctrine libraries, but that's a different topic.

greg0ire commented 2 years ago

Therefore, as soon as 2.2.0 is released and 2.2.x is the default branch, I don't think many people will read the old docs

Agreed, but we don't know when that will happen, do we? 2.2.x is not even created yet, so it's probably going to be a while, isn't it?

driehle commented 2 years ago

Agreed, but we don't know when that will happen, do we? 2.2.x is not even created yet, so it's probably going to be a while, isn't it?

Could be, though I hope it doesn't take long for 2.2.0 to be released. My PR for embeddables is from January...

TomHAnderson commented 2 years ago

2.2.x has been created

TomHAnderson commented 2 years ago

22 has been merged

TomHAnderson commented 2 years ago

@driehle I think everything is in line to get all this work merged. There are some notes here from @greg0ire review that need to be addressed. I'll be checking on this daily to get a new release.

driehle commented 2 years ago

@TomHAnderson if we are planning to release 2.2.0 soon, then I don't think @greg0ire asks for the docs to be merged in 2.1.x anymore. @greg0ire what do you say?

TomHAnderson commented 2 years ago

I don't want to make a 2.2 release until we've resolved as many of these PRs as we can.

greg0ire commented 2 years ago

@greg0ire what do you say?

I don't want to make your life harder than it needs to be, you can disregard my comment about the target branch.

greg0ire commented 2 years ago

@TomHAnderson if you could come on Slack to discuss the team, that would be great :slightly_smiling_face: