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

PSR-4 / xPDO 3.x Model Refactor #14534

Open
wants to merge 76 commits into
base: 3.x
from

Conversation

7 participants
@opengeek
Copy link
Member

commented Apr 5, 2019

What does it do?

Work in progress on xPDO 3.x model refactor of the Revo core featuring PSR-4 autoloading.

Why is it needed?

To allow the core model to be maintained via xPDO 3.x and to enable PSR-4 autoloading.

Related issue(s)/PR(s)

n/a

  • Finish converting core/model/modx/processors/ to core/src/Processors/
  • Convert sqlsrv model classes
  • Remove core/model/modx/*
  • Figure out what to do with manager/controllers to enable PSR-4 autoloading without losing ability to rename or relocate the manager/ directory
  • Work on recommended practices for extras to add their component src to the autoloader dynamically (see xPDO->addPackage() PSR-4 support)
  • Refactor unit tests for the new classes

@opengeek opengeek added this to the v3.0.0-alpha milestone Apr 5, 2019

@opengeek

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

There is a lot still to do on this…

@OptimusCrime

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

This looks great. I was just wondering if GitHub wrongfully reports new files instead of moved files? Are all these classes brand new, or are they just moved from their previous location in core?

@opengeek

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

They are being manually converted to their new location in core/src/ and then core/model/modx/ will only contain a few files to serve as faux class loaders for BC purposes.

@gpsietzema

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

Nice work! Am I correct in saying that this will just require a lot of time and most strategic thinking has been done? More people can now contribute towards this PR.

@JoshuaLuckers

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

Nice work! Am I correct in saying that this will just require a lot of time and most strategic thinking has been done? More people can now contribute towards this PR.

Yes, people are welcome to help convert the processors 👍 (without reformatting the code).

@Alroniks

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

Added search processor refactoring opengeek#4

@Alroniks

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

@JoshuaLuckers New processors are new files so I guess code reformatting also can be applied.

@JoshuaLuckers

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

Figure out what to do with manager/controllers to enable PSR-4 autoloading without losing ability to rename or relocate the manager/ directory

Can't we simply move manager/controllers to core/src/Manager? Various functions in modManagerController have to be changed to use the autoloader for the core classes of course.

@opengeek

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

@JoshuaLuckers I don't think they should be in the core at all, honestly. Perhaps we should just leave them as is for now. We can address them at least in a separate PR if the consensus is to convert them and add them to the autoloader dynamically based on the location of the manager folder. This also needs to be able to load theme specific controllers.

@matdave matdave added this to In progress in MODX 3.0.0-alpha Apr 10, 2019

@opengeek

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

I believe @Alroniks is working on the security processors.

@rthrash

This comment has been minimized.

Copy link
Member

commented May 3, 2019

This pull request has been mentioned on Community. There might be relevant details there:

https://community.modx.com/t/mab-2019-meeting-2-notes-may-2-2019/731/1

@opengeek opengeek marked this pull request as ready for review May 15, 2019

@opengeek opengeek requested a review from Mark-H as a code owner May 15, 2019

opengeek and others added some commits Mar 6, 2019

Alroniks and others added some commits May 4, 2019

@opengeek opengeek force-pushed the opengeek:psr4 branch from ab458ae to b01d2b7 May 15, 2019

@sergant210

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

Figure out what to do with manager/controllers to enable PSR-4 autoloading without losing ability to rename or relocate the manager/ directory

Connectors use core classes. But what to do with the core directory? That is the question.

"autoload": {
        "psr-4": {
            "MODX\\Revolution\\": "core/src/"
        },
        "classmap": [
            "core/xpdo/"
        ]
},
@Alroniks

This comment has been minimized.

Copy link
Collaborator

commented on setup/includes/config.core.php in 10e3975 May 17, 2019

Seems it is a reason why Travis can fail

This comment has been minimized.

Copy link
Member Author

replied May 18, 2019

Thanks. I thought I had that file marked with git update-index --assume-unchanged, but I guess that got reset somehow.

opengeek added some commits May 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.