module:composer-autoload command

[solomon-ochepa]

This PR focuses on adding the new autoload.psr-4 and autoload-dev.psr-4 data to modules composer.json file.

Usage

Updates the autoload data in the composer.json file for all modules.

php artisan module:composer-autoload --all

Updates the autoload data in the composer.json file of a specific module.

php artisan module:composer-autoload Blog

App path

Uses the value from modules.paths.app_folder.

[dcblogdev]

It would make sense if needed to update https://github.com/nWidart/laravel-modules/blob/master/src/Commands/ComposerUpdateCommand.php

As that updates composer.json.

@alissn thoughts on this?

Also when adding new functionality, we do need tests to ensure everything works, what is the reason you're unable to write tests?

[alissn]

Hi, I think it's not a good idea. This command updates all namespaces in the composer.json without changing anything in the classes. Migrating to a new structure should be done manually.

In my company's project, modules follow an old structure (see image below). If we run this command, what will happen? Nothing will work as expected.

This command cannot be applied to all modules because each one has its own structure and functionality.

The module:composerUpdate command only changes the composer.json files that contain "app" in the namespace. I'm not defending my code, but at that time, we needed to release a new major version. Now, we have more time to improve the code.

Additionally, the code needs to be more readable. Your code is very complicated.

[solomon-ochepa]

It would make sense if needed to update https://github.com/nWidart/laravel-modules/blob/master/src/Commands/ComposerUpdateCommand.php

As that updates composer.json.

I thought about that, but the goals of both commands are different.

• The module:composer-update command is focused on removing the App\\ path from the namespace,
• The module:composer-autoload command is precise and focused on creating/updating the autoload*.psr-4 part of the composer.json only, not the whole file. So, the two commands should exist separately as their purposes differ.

@alissn thoughts on this?

Also when adding new functionality, we do need tests to ensure everything works, what is the reason you're unable to write tests?

I may need assistance there. Each time I try to run a test and create one, it will all fail.

Usually, I'll clone the package inside my existing Laravel app, run the composer update command, and finally run the test or update snapshot.

When I run the composer test command

[solomon-ochepa]

Hi, I think it's not a good idea.

Why? Is it because it doesn't align with the code structure of your company? Are we developing this package solely for your company, or is it intended for broader use?

It's important to evaluate the benefits of updating your code and to avoid favouritism.

This pull request, unlike yours, is designed to function with all configurations, including default and custom app paths.

I acknowledge that modules can have varying file structures within a project. Instead of criticism, why not suggest or collaborate on a solution that accommodates these variations?

While your pull request may have had its shortcomings, we're working together to address them. This demonstrates teamwork, maturity, and a willingness to understand. Rather than simply pointing out flaws, let's focus on proposing solutions. It's essential to consider the perspectives of others, especially when making decisions that impact more than just yourself.

This command updates all namespaces in the composer.json without changing anything in the classes. Migrating to a new structure should be done manually.

I noticed that your module:composer-update command deliberately removed the App\\ path from every namespace in the composer.json file, but it seems the potential consequences weren't fully considered. It's understandable to question the reasoning behind such actions, especially when it appears that similar changes have been made previously without scrutiny.

However, it's important to recognize that this pull request offers a solution that addresses both of our concerns. It allows for compatibility with both custom and default configurations. With this implementation, you can maintain your custom structures, while others can continue to use the default configuration without encountering conflicts.

By collaborating on this solution, we can ensure that our codebase remains robust and adaptable to various project setups.

In my company's project, modules follow an old structure (see image below). If we run this command, what will happen? Nothing will work as expected.

If the command doesn't apply to your project, it's best not to run it. That way, you have the choice. When you changed everything in your previous pull request, you didn't think about how removing the 'App\' path would affect other companies' work.

In your company's project, modules follow the old structure, but in many other companies that are always updating, their modules use the new structure, like mine. So, why submit code that only helps your company instead of creating a solution that benefits everyone? That's what we're suggesting now.

This command cannot be applied to all modules because each one has its structure and functionality.

Absolutely! It's essential not to update all modules at once if it does not apply to your project.

Instead, you can utilize the php artisan module:composer-autoload Blog command to update a specific module individually.

Thanks for pointing this out! It's always good to have precise control over updates to avoid unnecessary disruptions.

The module:composerUpdate command only changes the composer.json files that contain "app" in the namespace.

You're correct. This command is optional and can only apply when we run it.

However, forcefully removing the App\ namespace from every namespace in the package codebase without consideration for individual module requirements was not the best decision. It's important to allow developers the autonomy to make such changes themselves, especially when it's relevant to their specific modules.

By enabling developers to determine when to remove the App\ namespace from their modules, we promote flexibility and ensure that changes align with the unique needs of each project. This approach empowers developers to manage their modules effectively and make adjustments that best suit their project requirements.

I'm not defending my code, but at that time, we needed to release a new major version. Now, we have more time to improve the code.

Absolutely! It's crucial to prioritize doing the right thing at all times. When submitting code changes, it's essential to consider the broader community and ensure that the code can benefit everyone, not just one company.

Our focus right now is on improving the codebase to enhance its usability and accessibility for all users.

Additionally, the code needs to be more readable. Your code is very complicated.

I follow one of the simplest principles of coding - KISS.

Nevertheless, could you please respond with a cleaner version of the code? I'd like to study your approach as well.

Thank you for the review.

[solomon-ochepa]

This command cannot be applied to all modules because each one has its own structure and functionality.

Added the --p|path option to enable custom app/ path per module/command

php artisan module:composer-autoload Blog --path=src

[alissn]

@solomon-ochepa

Why are you interested in writing an article for me? I'm just expressing my opinion as a user of this package.

Also, phpinsights is a very useful tool for checking code quality.

My time is very limited, but I'd be happy to help improve this package whenever I can.

Finally, thanks for spending time creating the PR. Best regards.

[solomon-ochepa]

This command cannot be applied to all modules because each has its structure and functionality.

Added the --p|path option to enable custom app/ path per module!

php artisan module:composer-autoload Blog --path=src

Hi, @dcblogdev and @alissn. I've added an option (--p|path) to specify the app/ folder per module!

With this new feature (--p|path), I believe we are now good to go.

[solomon-ochepa]

Hello, @dcblogdev why is this PR still pending?

It's an optional feature and will not impact the app by default!

Thank you.

[dcblogdev]

This needs tests to ensure it does what you think it does.

[dcblogdev]

since there are no tests I'm closing this PR.