search

nWidart / laravel-modules

Module Management In Laravel
https://docs.laravelmodules.com
MIT License
5.5k stars 954 forks source link

Update provider.stub #1926

Closed solomon-ochepa closed 3 weeks ago

alissn commented 1 month ago

Hi, @solomon-ochepa

Thank you for your contribution! However, the property names should follow the camelCase convention as per standard PHP and Laravel coding practices. Changing property names from camelCase to snake_case is not consistent with the rest of the Laravel codebase.

Please refer to the Laravel Framework repository for examples of this convention in practice.

Thanks for your understanding!

solomon-ochepa commented 1 month ago

Hi, @solomon-ochepa

Thank you for your contribution! However, the property names should follow the camelCase convention as per standard PHP and Laravel coding practices. Changing property names from camelCase to snake_case is not consistent with the rest of the Laravel codebase.

Please refer to the Laravel Framework repository for examples of this convention in practice.

Thanks for your understanding!

The link you shared doesn't specify any naming convention standard!

Laravel leverages PHP and PSR 4 standards and does not have a naming convention standard of its own.

PHP Standards:

PHP standard for function/method and variable/property naming is snake_case.

Nevertheless, every developer has a history and many come from another ecosystem such as Java, C++, Python, etc., and come with their inherited behaviors and principles, therefore we are given the freedom to use mixed naming conventions.

Therefore, the fact that you or the majority do a particular thing doesn't make it the right thing or the only way to do it.

User functions/methods naming conventions

image

variables/properties naming conventions

image

Bad method names

image

FYI: We have had this same discussion severally and you keep bringing it up. Instead, why not stick to what works for you? Sometimes in life, we might be doing the wrong thing, thinking is the right thing to do, however, a wise one should never shy away from corrections.

alissn commented 1 month ago

Please, please, please—before responding, take a moment to fully read and understand the comment. I clearly mentioned Property Name, not Method Name. All the references you've provided are related to method names, which isn't what I was addressing.

The link you shared doesn't specify any naming convention standard!
Laravel leverages PHP and PSR-4 standards and does not have a naming convention standard of its own.

The link I shared is from a package with 32K stars; you might have heard of it. I shared this package expecting that you would check a few classes, but it seems you didn't. To clarify, I'll provide some specific classes for your reference. If you truly believe your knowledge surpasses that of Taylor Otwell, Graham Campbell, Nuno Maduro, Dries Vints, and the entire Laravel core team, then by all means, submit a Pull Request to correct the framework.

However, if you find even one property in the Laravel framework using snake_case, I'll stop contributing to this package forever.

Here are a few examples directly from the Laravel source code:

I understand that each developer might have their own coding style, but when contributing to an open-source project, it's essential to follow the established code style of the package.

Lastly, I didn’t want to bring this up, but it's evident that your contributions are more about appearing on the top contributors' list rather than genuinely improving the package. You've ignored significant issues and performance considerations, focusing only on changing config files and stubs.

image

solomon-ochepa commented 1 month ago

Please, please, please—before responding, take a moment to fully read and understand the comment. I clearly mentioned Property Name, not Method Name. All the references you've provided are related to method names, which isn't what I was addressing.

PHP coding standards

PHP coding standards

Official PHP website referencing a past mistake and the link to the official coding standards.

PHP official

FYI: I'm not here for a fight. I appreciate your time and contributions!

alissn commented 1 month ago

@solomon-ochepa This discussion is about properties, not variables.

For variables, you can refer to the PHP documentation on variables. For properties, please see the PHP documentation on properties.

I’m not looking for your appreciation regarding my pull requests. Instead, I challenge you to find even one property in the Laravel framework that uses snake_case. If you can, please mention it in this Pull Request.

solomon-ochepa commented 1 month ago

@solomon-ochepa This discussion is about properties, not variables.

I understand that you often seem more focused on winning arguments than on understanding different perspectives. It's important to shift your mindset and argue to learn rather than only to prove a point.

When variables are defined in a Class, they are called properties, props, or fields.

For variables, you can refer to the PHP documentation on variables. For properties, please see the PHP documentation on properties.

Ref: For properties, please see the PHP documentation on properties.

Do you read the documentation before sharing the link? Class properties

You need to understand that there are differences between documentation, rules, and coding standards. What I shared are the rules and coding standards that guide everything in PHP!

I’m not looking for your appreciation regarding my pull requests. Instead, I challenge you to find even one property in the Laravel framework that uses snake_case. If you can, please mention it in this Pull Request.

I expect exactly that from you! Nevertheless, you should learn to appreciate people.

Can we please focus on the teamwork and collaboration that brought us here together and stop this childish, blind, selfish, and unfruitful argument for once?

solomon-ochepa commented 1 month ago

I've changed the variable name to camelCase as you recommended, I hope you are fine now.

Let's get on with other important things, please.

alissn commented 1 month ago

What is the benefit of changing variable names?

I believe moduleNameLower is more readable than nameLower, just as moduleName is clearer than simply name.

For this PR, I think updating only the docBlock would be sufficient.

On the other hand, this change isn't particularly important to me since we publish stub files, and this change won't affect our project. Ultimately, it's up to @dcblogdev to decide whether to merge or close this pull request.

I'm just bringing this to your attention for future contributions :)

solomon-ochepa commented 1 month ago

What is the benefit of changing variable names?

I believe moduleNameLower is more readable than nameLower, just as moduleName is clearer than simply name.

Clean and readable code;

On the other hand, this change isn't particularly important to me since we publish stub files, and this change won't affect our project.

Stop always focusing solely on your personal interests. This is an open-source project used by hundreds of people.

For your information, it's advisable to re-publish the stubs and config after every upgrade to stay aligned with ongoing development.

alissn commented 1 month ago

I'm not focusing on personal interests; I always consider the bigger picture, which you seem unwilling to understand.

For your information, it's advisable to re-publish the stubs and config after every upgrade to stay aligned with ongoing development.

You're the only one changing the stubs file without any functional improvement, just stylistic changes.

Keep your advice for yourself. For instance, your latest pull request reflects something we've already had in our project for two years, and it's already been cleaned up.

image

Have a good day. I don't have time to argue and try to convince you.🙂

dcblogdev commented 3 weeks ago

Thank you both, I agree with using camelCase for variable names. I'm find with using $name.