search

themehybrid / hybrid-core

Official repository for the Hybrid Core WordPress development framework.
GNU General Public License v2.0
687 stars 143 forks source link

Move ServiceProvider to Core? #187

Closed justintadlock closed 3 years ago

justintadlock commented 3 years ago

Currently, the ServiceProvider class is in hybrid-tools. However, the Tools project is probably better fitted for things like its current Collection class (thinking of adding some extra string and array classes to that in the future).

ServiceProvider will always be deeply connected to Core/Application. It probably makes sense they are in the same repo.

Any thoughts? Objections to the change?

saas786 commented 3 years ago

Sure, ServiceProvider is certainly deeply connected to the hybrid-core, no object to move it to core project.

But keeping it in hybrid-tools is fine too. As its a required dependency anyway. Though hybrid-tools can have more loosely connected tools etc.., doesn't mean things like ServiceProvider doesn't belong in there.

justintadlock commented 3 years ago

Sounds good. I'll try to make that change tonight. I'll probably do a soft deprecation of Hybrid\Tools\ServiceProvider since so many packages rely on it right now, at least until everything is moved over and before the 6.0 release.

The only remaining question might be the namespace/folder. Under Hybrid\Core or something new like Hybrid\Support?

Note: Other than ServiceProvider, hybrid-tools is only a dependency because of the collect() helper function. We should probably move that function to the hybrid-tools project.

saas786 commented 3 years ago

Sounds good. I'll try to make that change tonight. I'll probably do a soft deprecation of Hybrid\Tools\ServiceProvider since so many packages rely on it right now, at least until everything is moved over and before the 6.0 release.

The only remaining question might be the namespace/folder. Under Hybrid\Core or something new like Hybrid\Support?

Note: Other than ServiceProvider, hybrid-tools is only a dependency because of the collect() helper function. We should probably move that function to the hybrid-tools project.

Yes, deprecation can be set. :)

Maybe Hybrid\Contracts can be added to: hybrid-core/src/Contracts

Yes, make sense to move collect() to hybrid-tools as well.