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

Change Zend namespace to Laminas #1963

Merged
merged 4 commits into from Jan 6, 2020

Conversation

@tankerkiller125
Copy link
Member

tankerkiller125 commented Jan 4, 2020

Fixes #0000

Changes proposed in this pull request:

Changes Zend namespace to Laminas to reflect the recent name change of the Zend Framework

Reviewers should focus on:

  • Did I find all the Zend usages?

Screenshot

Confirmed

- [ ] Frontend changes: tested on a local Flarum installation.

  • Backend changes: tests are green (run composer test).
@franzliedke

This comment has been minimized.

Copy link
Member

franzliedke commented Jan 5, 2020

Thanks for tackling this!!!

We do have a small problem, though, and that is extensions using some Diactoros classes. They would have to switch to Laminas to be compatible with the next beta - which is something we are trying to avoid - for one beta cycle. Unfortunately, not even our bundled extensions have explicitly declared the dependency - they just trust that flarum/core will include it.

Is it possible to load both dependencies? We can then use Laminas everywhere in our code, and remove the old Zend dependencies for beta.13. That will give extensions one release cycle time to switch over, with both the new and old code working.

@luceos luceos added this to the 0.1.0-beta.12 milestone Jan 5, 2020
@datitisev

This comment has been minimized.

Copy link
Member

datitisev commented Jan 5, 2020

@franzliedke Laminas has a package that should allow the use of ZendFramework classes with Laminas, see laminas/laminas-zendframework-bridge

@tankerkiller125

This comment has been minimized.

Copy link
Member Author

tankerkiller125 commented Jan 5, 2020

@datitisev @franzliedke Based on reading through that code package it appears that we should be able to add it as a composer dependency along with the Laminas changes proposed here and it should protect backwards compatibility. I'll see if I can test tonight or tomorrow to ensure that that statement is correct.

Ensure backwards compatibility for extensions that use the Zend framework but don't explicitly require it.
@tankerkiller125

This comment has been minimized.

Copy link
Member Author

tankerkiller125 commented Jan 6, 2020

I've added the Laminas bridge as a dependency in core, it works exactly as expected (generating autoload classes that just reference Laminas) this should resolve any backwards compatibility issues for beta.12 and the package can then be removed in beta.13 (breaking the Zend namespace) this is definitely something we will want to make sure to publish in the changelog for developers.

Copy link
Member

franzliedke left a comment

Great, thank you - especially for taking care of backwards compatibility!

@franzliedke franzliedke merged commit d7a5a6a into flarum:master Jan 6, 2020
10 checks passed
10 checks passed
PHP 7.1 / MySQL
Details
PHP 7.1 / MariaDB
Details
PHP 7.2 / MySQL
Details
PHP 7.2 / MariaDB
Details
PHP 7.3 / MySQL
Details
PHP 7.3 / MySQL (prefix)
Details
PHP 7.3 / MariaDB
Details
PHP 7.3 / MariaDB (prefix)
Details
WIP Ready for review
Details
continuous-integration/styleci/pr The analysis has passed
Details
@franzliedke

This comment has been minimized.

Copy link
Member

franzliedke commented Jan 6, 2020

I see the following follow-ups:

  • a ticket for removing the BC layer, targeted for beta.13
  • update bundled extensions to use the new namespaces
    • auth-facebook
    • auth-github
    • auth-twitter
    • pusher
    • tags
  • update the docs: docs/extend/routes.md references old namespace

@tankerkiller125 Can you take care of these, too? 🙏

@tankerkiller125

This comment has been minimized.

Copy link
Member Author

tankerkiller125 commented Jan 7, 2020

@franzliedke I'll take care of those, labels/milestone for the ticket will have to be added by someone else (no access) #1964

Edit: Pull request for all extensions and the docs have been created, all are titled "Change Zend to Laminas"

@tankerkiller125 tankerkiller125 deleted the tankerkiller125:mk/zend-to-laminas branch Jan 7, 2020
@luceos

This comment has been minimized.

Copy link
Member

luceos commented Jan 7, 2020

Milestone modified for #1964

@franzliedke

This comment has been minimized.

Copy link
Member

franzliedke commented Jan 7, 2020

I like how all of these extension PRs had ever-so-slightly different description texts. 🤣

In all seriousness: thanks for taking care of this, and so swiftly!

luceos added a commit that referenced this pull request Feb 4, 2020
Also ensure backwards compatibility for extensions that use the Zend framework but don't explicitly require it.
wzdiyb added a commit to wzdiyb/core that referenced this pull request Feb 16, 2020
Also ensure backwards compatibility for extensions that use the Zend framework but don't explicitly require it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.