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

Chore: titleCase folders #930

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

ryanmitchell
Copy link
Member

Its a small thing but it makes a big difference :)

Ideally at the same time we could move

-> views and assets into resources
-> all other folders apart from resources, language and database into a src folder, however this doesn't seem to work with Laravel autoloading.

sampoyigi and others added 14 commits December 2, 2021 19:41
Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
* wip

Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>

* wip

Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
…ways be combined by default

Signed-off-by: Sam Poyigi <6567634+sampoyigi@users.noreply.github.com>
@ryanmitchell
Copy link
Member Author

The main reason behind doing this is to make the files PSR-4 compatible, which the currently aren't. If you try and include the 'modules' into any other application it fails due to them not complying with PSR-4 autoloading, which makes the module approach redundant.

@sampoyigi
Copy link
Member

Without a doubt, this has advantages, but I'm more concerned about the gotchas (as I noted on Discord). I'm willing to reconsider once we've worked out the kinks.

Ideally at the same time we could move
-> views and assets into resources
-> all other folders apart from resources, language and database into a src folder, however this doesn't seem to work with Laravel autoloading.

Moving views and assets into resources means we can't have the modules as standalone packages. I don't see the issue with having the views and assets in their current location when we'll be using a public folder and assets combiner.

@ryanmitchell
Copy link
Member Author

Without the changes (so as things stand) you cant actually include the packages on another project which means splitting them out as modules has no benefit.

My hope had been we would get to the stage where they were each available with their own service provider, as per other laravel packages, so they could be used alongside other installed packages, for example a CMS platform for a marketing site.

@sampoyigi
Copy link
Member

Even with the proposed changes, making the modules work independently will require more work in the codebase as things stand they are heavily dependent on each other.

Anyway, as I previously stated, I'm fine with sticking to PS4 standards as long as it doesn't cause any problems. Allow me to conduct some tests to clear my concerns.

@ryanmitchell
Copy link
Member Author

Im not so bothered with making them work independently of each other, but making the whole codebase a package so it can exist alongside other packages inside an app.

I know we've chatted about this before, but to me the default install should be a clean laravel install with tasty stuff in a vendor folder, like how Statamic do it: https://github.com/statamic/statamic . I know theres a lot to get there and we dont need to do it all at once, but we need to move towards it as its the most developer friendly experience.

@sampoyigi sampoyigi changed the base branch from 4.0 to develop March 1, 2022 15:07
@sampoyigi
Copy link
Member

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this is still being worked on, please respond and we will re-open this pull request. If this pull request is critical to your business, consider the Dedicated Support Service where a Service Level Agreement is offered.

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.

None yet

2 participants