Skip to content

Open 2.0 - PHP 7.2, change namespace#193

Merged
lcobucci merged 3 commits intodoctrine:masterfrom
Majkl578:dev/open-2.0
May 7, 2018
Merged

Open 2.0 - PHP 7.2, change namespace#193
lcobucci merged 3 commits intodoctrine:masterfrom
Majkl578:dev/open-2.0

Conversation

@Majkl578
Copy link
Copy Markdown
Contributor

  • Open 2.0
  • Require PHP 7.2
  • Change namespace to Doctrine\Annotations

I'd like to make this PR a starting point for upcoming Annotations 2.0, gradually reviving #75. 🎉
It won't be as easy as it may look, it is not up to date with master, also is a bit older - built back in pre-7.1 era. Thus I would like to split it into smaller chunks - here and in next PRs.

1.7/1.8 would be the last 1.x release shipping compatiiblity layer for 2.0 (similar to ORM 2.7 for 3.0).

@@ -17,7 +17,7 @@
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\Common\Annotations;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this will be a massive BC break. In BetterReflection, we had introduced a shim for this: https://github.com/Roave/BetterReflection/blob/44ef19dc4544a0673a00a080a2d3a1c60d5ce42b/namespace-bc-aliases.php

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See:

1.7/1.8 would be the last 1.x release shipping compatiiblity layer for 2.0

It will be a BC break, on the other hand this is new major after 5+ years so changes are to be expected, this will not be the only BC issue. Also we want to get rid of the "Common" thing, right? 😉

Either way I am not sure whether the polyfill should go only to 1.x or 2.x too. There is plenty of time to discuss that and do properly before release.

Copy link
Copy Markdown
Member

@lcobucci lcobucci Apr 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPUnit's strategy wen't well IMO... namespaces got added in 6.x and compatibility layer added in 5.x.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is if we change the public API so much that the class aliasing might not be that helpful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we change the public API so much that the class aliasing might not be that helpful

That's true and needs to be considered, but let's keep that for discussion in specific PRs. Nothing apart namespaces is changed here. 😊

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius We've spoken about this before (re: BC on namespace change) back at True North 2016 and we agreed to remove Common. =P

The motivation is that Annotations is a widely spread enough that even though makes it harder to make BC breaks, it's also a project that lives outside of Doctrine\Common umbrella. We originally defined the Common slot as support tools for ORM, but clearly it went further than we imagined. It's time to drop Common namespace.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about removing the Common part of the namespace, but only if you provide a BC layer to allow for projects to support both 1.x and 2.x in their code requiring a Reader.
Due to 2.0 supporting only PHP 7.2+, switching to doctrine/annotations 2.0 only for Symfony is not an option (as Symfony 4 supports 7.1+). This means we need to be able to support both 1.x and 2.x in our codebase

@Majkl578 Majkl578 added this to the v2.0.0 milestone May 7, 2018
@lcobucci lcobucci self-assigned this May 7, 2018
@lcobucci lcobucci merged commit 31f4c4a into doctrine:master May 7, 2018
@Majkl578 Majkl578 deleted the dev/open-2.0 branch May 7, 2018 23:00
@Majkl578 Majkl578 mentioned this pull request Apr 3, 2019
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants