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

Update provider.stub #1926

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Update provider.stub #1926

merged 3 commits into from
Sep 10, 2024

Conversation

solomon-ochepa
Copy link
Contributor

No description provided.

@alissn
Copy link
Contributor

alissn commented Aug 29, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

alissn commented Aug 30, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

alissn commented Aug 31, 2024

@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
Copy link
Contributor Author

solomon-ochepa commented Aug 31, 2024

@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
Copy link
Contributor Author

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
Copy link
Contributor

alissn commented Aug 31, 2024

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
Copy link
Contributor Author

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;

  • Avoid the repetition of keywords: the entire concept here is the module and we all know that already, there is no need to prefix everything repeatedly with the keyword module.
  • KISS: Always use short and precise names over lengthy names. Avoid spaghetti names as much as possible!
  • Module Design: module is the concept, and name is a property; module -> name instead of module -> moduleName

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
Copy link
Contributor

alissn commented Aug 31, 2024

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
Copy link
Collaborator

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

@dcblogdev dcblogdev merged commit 2890081 into nWidart:master Sep 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants