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

Fix naming: doesntEmpty -> isNotEmpty #289

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

djlimix
Copy link
Contributor

@djlimix djlimix commented Sep 7, 2024

This PR creates new method called isntEmpty, which uses the same logic as method doesntEmpty. Gramatically, doesntEmpty is not correct, therefore it is also not intuitive (why have one method named isEmpty and the opposite named doesntEmpty).

The original method doesntEmpty still exists (to ensure backwards-compatibility), but is deprecated.

I understand that this is not an important change, but I feel like it is something that could be changed.

@andrey-helldar
Copy link
Member

I agree that the naming may be incorrect. This is due to the fact that English is not my native language.

The name isntEmpty looks more correct from the grammatical point of view, but what do you say if we call the method isNotEmpty?

For example, like this:

@andrey-helldar andrey-helldar added the added Change that adds something label Sep 7, 2024
@djlimix
Copy link
Contributor Author

djlimix commented Sep 7, 2024

I agree and I've changed the method name to isNotEmpty to follow Laravel naming.

@andrey-helldar andrey-helldar changed the title Fix naming: doesntEmpty -> isntEmpty Fix naming: doesntEmpty -> isNotEmpty Sep 7, 2024
@andrey-helldar
Copy link
Member

andrey-helldar commented Sep 7, 2024

While this package may work outside of the Laravel ecosystem, it was created with that in mind, but that said, many of the titles I looked at are there.

Thanks for your contribution! There's a release coming up :)

@andrey-helldar andrey-helldar merged commit 40f3576 into TheDragonCode:main Sep 7, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
added Change that adds something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants